[PATCH] D152505: [Clang] Directly create opaque pointers

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG09d6ee765780: [Clang] Directly create opaque pointers (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Re

[PATCH] D152321: [clang] Replace use of Type::getPointerTo() (NFC)

nikic added a comment. Looks reasonable. Main note I have is that I think the use of getUnqual() over passing AddrSpace=0 would be preferred. Comment at: clang/lib/CodeGen/CGAtomic.cpp:91 auto Addr = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast( -VoidPt

[PATCH] D152321: [clang] Replace use of Type::getPointerTo() (NFC)

nikic accepted this revision. nikic added a comment. LGTM -- do you have commit access, or should someone commit this for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152321/new/ https://reviews.llvm.org/D152321 _

[PATCH] D152999: [CGTypes] Remove recursion protection

This revision was automatically updated to reflect the committed changes. Closed by commit rG2903a8ab0726: [CGTypes] Remove recursion protection (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D148216: Add support for annotations in UpdateTestChecks (NFC)

nikic added a comment. Could you please rebase the patch to current main? I wasn't able to apply it. The behavior you describe sounds reasonable to me. Comment at: llvm/utils/UpdateTestChecks/common.py:1286 + if value == default_value: +continue if action.des

[PATCH] D153196: [clang] Replace uses of CGBuilderTy::CreateElementBitCast (NFC)

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/CodeGen/CGObjC.cpp:2207 llvm::Type *origType = addr.getElementType(); - addr = CGF.Builder.CreateElementBitCast(addr, CGF.Int8PtrTy); + addr = a

[PATCH] D139274: Store OptTable::Info::Name as a StringRef

nikic added a comment. Build failure in pre-merge checks: FAILED: lib/Option/CMakeFiles/LLVMOption.dir/OptTable.cpp.o CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++ -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACRO

[PATCH] D139274: Store OptTable::Info::Name as a StringRef

nikic added inline comments. Comment at: llvm/lib/Option/OptTable.cpp:44 if (a == '\0') return 0; The code in this function (and also StrCmpOptionName) depends on the terminating null byte, while a StringRef is not guaranteed to by directly follow

[PATCH] D138722: Overload all llvm.annotation intrinsics for globals argument

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/test/Bitcode/upgrade-annotation.ll:14 + +declare i32 @llvm.annotation.i32(i32, i8*, i8*, i32) +; CHECK: declare i32 @llvm.annotation.i32.p0i8(i32, i8*, i8

[PATCH] D139274: Store OptTable::Info::Name as a StringRef

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LG Comment at: llvm/lib/Option/OptTable.cpp:41 + size_t MinSize = std::min(A.size(), B.size()); + if (int res = A.substr(0, MinSize).compare_insensitive(B.substr(0, MinSize))

[PATCH] D139450: Warn about unsupported ibmlongdouble

nikic added a reviewer: PowerPC. nikic added a comment. Can you please upload the patch with full context (`-U9`)? The assumption here is that `libc++` is being compiled with a compiler that has the same ieeelongdouble default as the clang that is being built, right? I guess that's the most

[PATCH] D139507: [Intrinsic] Add get.rounding as alias to flt.rounds and rename related DAG nodes

nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. In D139507#3980555 , @qiucf wrote: > In D139507#3978449 , @sepavloff > wrote: > >> Thank you for wor

[PATCH] D139450: Warn about unsupported ibmlongdouble

nikic added inline comments. Comment at: clang/lib/Driver/ToolChains/PPCLinux.cpp:74 +const Driver &D, +const llvm::opt::ArgList &Args) const { + if (Args.hasArg(options::OPT_nostdlib, options::OPT_nostdlibxx)) I don't think this formatting is right. You

[PATCH] D135269: [AMDGPU] Disable bool range metadata to workaround backend issue

nikic added a comment. Checking back here again on whether there is any progress on finding the root cause of the issue. If no progress is expected in the near future, I'd ask for this patch to be reverted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D134410: [clang][CodeGen] Add noundef metadata to load instructions (preliminary 1 or 5)

nikic added a comment. In D134410#3898615 , @nlopes wrote: > In D134410#3898563 , @nikic wrote: > >> FWIW, I don't agree with this. It's fine to change load semantics, as long >> as bitcode autoupgrade is handled

[PATCH] D139745: Use poison instead of undef where its used as placeholder[NFC]

nikic added a comment. The struct/insertvalue changes here are fine (no change in semantics). Can't comment on the vector/insertelement changes, you might want to split them out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139745/new/ https://re

[PATCH] D131830: [OpenMP] Clang Support for taskwait nowait clause

nikic added inline comments. Comment at: clang/test/OpenMP/taskwait_depend_nowait_codegen.cpp:1 +// RUN: %clang_cc1 -no-opaque-pointers -verify -triple x86_64-apple-darwin10 -fopenmp -fopenmp-version=51 -x c++ -emit-llvm %s -o - | FileCheck %s +// RUN: %clang_cc1 -no-opaque-poin

[PATCH] D139006: [UpdateTestChecks] Match define for labels

nikic added a comment. In D139006#3988227 , @sebastian-ne wrote: > In D139006#3988215 , @mkazantsev > wrote: > >> So now every single test needs to be regenerated? It'll create straw diff >> from nowhere... > >

[PATCH] D139507: [Intrinsic] Rename flt.rounds intrinsic to get.rounding

nikic added inline comments. Comment at: libcxxabi/test/test_demangle.pass.cpp:12917 {"_ZNK4llvm17X86TargetLowering15LowerTRAMPOLINEENS_7SDValueERNS_12SelectionDAGE", "llvm::X86TargetLowering::LowerTRAMPOLINE(llvm::SDValue, llvm::SelectionDAG&) const"}, - {"_ZNK4llvm1

[PATCH] D139507: [Intrinsic] Rename flt.rounds intrinsic to get.rounding

nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. (per previous comment, mostly looks good though) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139507/new/ https://reviews.llvm.org/D139

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable

nikic added a comment. In D125291#3548671 , @jyknight wrote: > So, I've been spending some significant time looking into this. I found that > I couldn't really review this change for correctness, because I find it > basically impossible to figure out wh

[PATCH] D139507: [Intrinsic] Rename flt.rounds intrinsic to get.rounding

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139507/new/ https://reviews.llvm.org/D139507 ___ c

[PATCH] D139006: [UpdateTestChecks] Match define for labels

nikic added a comment. In D139006#384 , @jdoerfert wrote: > Why not just `--function-define` as a way to enable the `define` but not the > `captures`, if that is really something necessary. > It is unclear to me why `function-signature` is not suffic

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

nikic added inline comments. Comment at: clang/lib/CodeGen/Address.h:30 + // Int portion stores lower 3 bits of the log of the alignment. + llvm::PointerIntPair ElementType; Are we guaranteed 3 bits even on 32-bit architectures? Comment at:

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

nikic added inline comments. Comment at: clang/lib/CodeGen/Address.h:45 +auto AlignLog = llvm::Log2_64(alignment.getQuantity()); +assert(AlignLog < (1 << 6) && "cannot fit alignment into 6 bits"); +Pointer.setInt(AlignLog >> 3); nikic wrote: > Why can

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

nikic added inline comments. Comment at: clang/lib/CodeGen/Address.h:30 + // Int portion stores lower 3 bits of the log of the alignment. + llvm::PointerIntPair ElementType; dblaikie wrote: > aeubanks wrote: > > nikic wrote: > > > Are we guaranteed 3 bits eve

[PATCH] D117600: [CGCall] Annotate operator new with inaccessiblememonly if AssumeSaneOperatorNew is on

nikic added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2071 RetAttrs.addAttribute(llvm::Attribute::NoAlias); + FuncAttrs.addAttribute("inaccessiblememonly"); +} This should be `Attribute::InacccessibleMemOnly`. It's not

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

nikic added a comment. Approach looks reasonable to me. Comment at: clang/lib/CodeGen/Address.h:29 +// so we fallback to storing the alignment separately. +template = 8> class AddressImpl {}; + Why do we need the extra T parameter? Comment at

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LG from my side Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117262/new/ https://reviews.llvm.org/D117262 __

[PATCH] D118313: [Driver] Remove -fno-experimental-new-pass-manager

nikic added a comment. So this just forces you to change `-fno-experimental-new-pass-manager` to `-flegacy-pass-manager`? I'm not sure I see the point. Note that removal of middle-end LegacyPM functionality is currently blocked on https://reviews.llvm.org/D98481 landing, which prevents NewPM de

[PATCH] D118313: [Driver] Remove -fno-experimental-new-pass-manager

nikic added inline comments. Comment at: clang/docs/ReleaseNotes.rst:112 + pass manager for the optimization pipeline was deprecated in 13.0.0 and will + be removed after 14.0.0. Would be good to mention `-flegacy-pass-manager` as the alternative here. Repo

[PATCH] D118385: [NFC] Optimize FoldingSet usage where it matters

nikic added a comment. It might make sense to split this into individual changes, so it's clearer what impact each of them has. I tested just moving the `AddXYZ` methods into the header, which had a large positive impact: https://llvm-compile-time-tracker.com/compare.php?from=784e01abca65722df

[PATCH] D116492: [lld] Deprecate using llvm-config to detect llvm installation

nikic added inline comments. Comment at: lld/CMakeLists.txt:14 + set(LLVM_CONFIG_OUTPUT) + if(LLVM_CONFIG) +set (LLVM_CONFIG_FOUND 1) Is it intentional that lld now requires you to set LLVM_CONFIG instead of LLVM_CONFIG_PATH? Based on the deprecation, I wo

[PATCH] D116492: [lld] Deprecate using llvm-config to detect llvm installation

nikic added a comment. Would it be possible to share a complete new-style cmake invocation that is supposed to work? If I only point LLVM_DIR to the cmake directory, then LLVM_MAIN_SRC_DIR ends up being empty (it is set to MAIN_SRC_DIR, which, as far as I can tell, is only initialized in the LL

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

nikic added a comment. Because I happened to also run into this, reduced IR for `-mtriple=riscv32 -mattr=+d` is: define float @test(float %x) { %1 = tail call float asm sideeffect alignstack "mv a0, a0", "={x10},{x10}"(float 0.00e+00) ret float 0.00e+00 } Repository: rG

[PATCH] D120290: [Clang][OpenMP] Add the codegen support for `atomic compare capture`

nikic added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6067 + if (VPtr) { +VOpVal = {VPtr, VPtr->getType()->getPointerElementType(), + V->getType().isVolatileQualified(), Please avoid adding new calls to getPointerElementTyp

[PATCH] D120334: [NFC][Lexer] Make Lexer::LangOpts const reference

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. Looks reasonable to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120334/new/ https://reviews.llvm.org/D120334 __

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

nikic added a comment. Hm, it looks like enabling PIE has a pretty big negative compile-time impact, with a 20% regression on sqlite3: http://llvm-compile-time-tracker.com/compare.php?from=611122892e6d5813444bdd0e1fbe0a96f6e09779&to=3c4ed02698afec021c6bca80740d1e58e3ee019e&stat=instructions Cod

[PATCH] D120527: [OpaquePtr][AArch64] Use elementtype on ldxr/stxr

nikic added a comment. An alternative I suggested on https://github.com/llvm/llvm-project/issues/51165 was to overload these intrinsics by result/value type. I think that would be cleaner design-wise, but it also seems harder to implement (requires custom ISD nodes), so I'm okay with the elemen

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

nikic added a comment. @MaskRay Please revert the change and all dependent changes you have made. A revert is not a personal affront to you. It's not a judgement that you or your change are bad. It's a simple matter of policy and standard procedure. There's a good chance that next week you'll g

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

nikic added a comment. Yes, because you reverted the change for that one buildbot, of course it is green now. You could have also made the buildbot green by disabling tests on that bot. Or disabling sanitizers on it. Doesn't change the fact that the configuration it was originally testing is st

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

nikic added a comment. In D120305#3347192 , @tstellar wrote: > In D120305#3347177 , @nikic wrote: > >> Yes, because you reverted the change for that one buildbot, of course it is >> green now. You could have also

[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

nikic added a comment. I've been looking into the compile-time regressions. The large regression on sqlite3 is due to differences in inlining behavior. The inlining cost model is slightly different for PIC builds, because GEPs that are based on a global and have a variable offset are no longer

[PATCH] D106184: [BPF] Use elementtype attribute for preserve.array/struct.index intrinsics

nikic added inline comments. Herald added a project: All. Comment at: llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp:303 CInfo.RecordAlignment = DL->getABITypeAlign(CInfo.Base->getType()->getPointerElementType()); return true; yonghong-song wr

[PATCH] D101017: [NewPM] Make GlobalsAA available earlier in the pipeline

nikic added a comment. @aeubanks It seems like this has non-trivial impact on code size (http://llvm-compile-time-tracker.com/compare.php?from=a62cbd9a0211d08bace8794b435996890feb44d4&to=7805d7f72c337bfcc2fbc2dc9d2b9ac23474d5d9&stat=size-text), so probably the compile-time change is a side-effec

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

nikic added inline comments. Comment at: llvm/test/Transforms/InstCombine/logical-select.ll:385 +; CHECK-NEXT:[[OR:%.*]] = select i1 [[AND1]], i1 true, i1 [[AND2]] +; CHECK-NEXT:ret i1 [[OR]] ; It looks like this fold could be salvaged, if we wanted to:

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

nikic added inline comments. Comment at: llvm/test/Transforms/InstCombine/logical-select.ll:385 +; CHECK-NEXT:[[OR:%.*]] = select i1 [[AND1]], i1 true, i1 [[AND2]] +; CHECK-NEXT:ret i1 [[OR]] ; aqjune wrote: > nikic wrote: > > It looks like this fold cou

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

nikic added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll:20 +; FIXME: noundef should be attached to args define i1 @will_not_overflow(i64 %arg, i64 %arg1) { spatel wrote: > aqjune wrote: > > aqjune wrote:

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

nikic added a comment. Went through all the tests again, looks like there's three patterns that could still be handled. I think the last one of them should be handled, and for the other two it's fine to just open an issue. Comment at: llvm/test/Transforms/InstCombine/and2.ll:

[PATCH] D100917: [NewPM] Only invalidate modified functions' analyses in CGSCC passes

nikic added a comment. An unfortunate side-effect of this change is that NewPM uses even more memory. tramp3d-v4 is up 20% in max-rss (https://llvm-compile-time-tracker.com/compare.php?from=4ef1f90e4d564b872e3598ccef45adb740eb0f0d&to=d14d84af2f5ebb8ae2188ce6884a29a586dc0a40&stat=max-rss) Repos

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM. I can take care of reverting if there are issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101191/new/ https://reviews.llvm.org/D101191

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

nikic added a comment. In D101191#2751002 , @sanwou01 wrote: > Hi, we've got a ~6% regression in SPEC INT 2006 462.libquantum on AArch64 > (both -flto and -Ofast) that comes back to this change. See here for a > reproducer https://godbolt.org/z/dq98Gqqx

[PATCH] D115003: [funcattrs] Infer writeonly argument attribute [part 2]

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:728 + } else if (CB.hasFnAttr(Attribute::WriteOnly) || + CB.dataOperandHasImpliedAttr(UseIndex,

[PATCH] D116587: [ConstantFold] Remove unnecessary bounded index restriction

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd74212987b35: [ConstantFold] Remove unnecessary bounded index restriction (authored by nikic). Herald added a project: clang. Herald added a subscrib

[PATCH] D116599: Simplify AttrBuilder storage for target dependent attributes

nikic added a comment. I'm generally on board with this change. After D116110 , the `AttrBuilder` is only used in situations where we actually expect all the added attributes to be converted to `Attribute`s in any case, so we don't lose anything by storing them

[PATCH] D116599: Simplify AttrBuilder storage for target dependent attributes

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LG from my side, but I'd like a second opinion for the "require LLVMContext in AttrBuilder" part of the change, as that's the main API impact. Comment at: llvm/lib/AsmParser/L

[PATCH] D116666: [CodeGen] Emit elementtype attributes for indirect inline asm constraints

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe8b98a5216db: [CodeGen] Emit elementtype attributes for indirect inline asm constraints (authored by nikic). Herald added a project: clang. Herald ad

[PATCH] D116856: [docs] Fix documentation of -fno-strict-float-cast-overflow after D115804.

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116856/new/ https://reviews.llvm.org/D116856 ___ c

[PATCH] D116935: [IRBuilder] Introduce folder using inst-simplify, use for Or fold.

nikic added a comment. In D116935#3231615 , @fhahn wrote: > In D116935#3231477 , @nikic wrote: > >> Why do we need / want to use the InstSimplifyFolder in SROA? > > I don't have any strong opinions either way. The

[PATCH] D116935: [IRBuilder] Introduce folder using inst-simplify, use for Or fold.

nikic accepted this revision. nikic added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116935/new/ https://reviews.llvm.org/D116935 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D116219: [CodeGen] Make element type in emitArrayDestroy() predictable

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2d1b55ebea88: [CodeGen] Make element type in emitArrayDestroy() predictable (authored by nikic). Herald added a project: clang. Herald added a subscr

[PATCH] D117431: [IRBuilder] Migrate and-folding to value-based FoldAnd.

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117431/new/ https://reviews.llvm.org/D117431 ___ c

[PATCH] D115630: [CodeGen] Require use of Address::invalid() for invalid address

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb8d121eb1d61: [CodeGen] Require use of Address::invalid() for invalid address (NFC) (authored by nikic). Herald added a project: clang. Herald added

[PATCH] D115630: [CodeGen] Require use of Address::invalid() for invalid address

nikic added inline comments. Comment at: clang/lib/CodeGen/Address.h:79 class ConstantAddress : public Address { + ConstantAddress(nullptr_t) : Address(nullptr) {} + yubing wrote: > Has anyone encountered buildfail due to missing "std" before nullptr_t? Yeah, t

[PATCH] D115725: [CodeGen] Store ElementType in Address

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGabbc2e997bbf: [CodeGen] Store ElementType in Address (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Rep

[PATCH] D115791: [CodeGen] Store ElementType in LValue

nikic created this revision. nikic added reviewers: opaque-pointers, rjmccall. nikic requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Store the pointer element type inside LValue so that we can preserve it when converting it back into an Ad

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

nikic added a comment. I like the general direction. A possible reframing would be SmallAttrBuilder -> MutableAttributeSet. I think my main question here would be in which contexts we still use / want to use the old AttrBuilder? Comment at: llvm/include/llvm/IR/Attributes.h:9

[PATCH] D115804: [CodeGen] use saturating FP casts when compiling with "no-strict-float-cast-overflow"

nikic added a comment. Looks reasonable. Making float to int cast well defined is exactly why these intrinsics exist. Should we also drop the "string-float-cast-overflow" attribute, as UB is now prevented in a different way? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115804/new/ ht

[PATCH] D115791: [CodeGen] Store ElementType in LValue

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6bca9a428e32: [CodeGen] Store ElementType in LValue (authored by nikic). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D115804: [CodeGen] use saturating FP casts when compiling with "no-strict-float-cast-overflow"

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM In D115804#3195158 , @spatel wrote: > In D115804#3195002 , @nikic wrote: > >> Looks reasonable. Making fl

[PATCH] D115924: [ConstantFolding] Unify handling of load from uniform value

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9fd4f80e33a4: [ConstantFolding] Unify handling of load from unifor

[PATCH] D115924: [ConstantFolding] Unify handling of load from uniform value

nikic added a comment. @paulwalker-arm Thanks for the report, I've reverted the commit for now. I've put up D115994 to either fix the test or be told that clang generates incorrect initialization code, I'm not completely sure which it is. Repository: rG LL

[PATCH] D115924: [ConstantFolding] Unify handling of load from uniform value

nikic added a comment. In the meantime, I've applied https://github.com/llvm/llvm-project/commit/2926d6d335aca7e3f57ac45e6b25b1716e053fb3 as a targeted fix for the original assertion failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115924/n

[PATCH] D116110: Introduce the AttributeMask class

nikic added inline comments. Comment at: llvm/include/llvm/IR/Attributes.h:956 +assert(Attribute::isEnumAttrKind(Val) && + "Adding integer/type attribute without an argument!"); +Attrs[Val] = true; This assert doesn't make sense to me. Attribute

[PATCH] D116110: Introduce the AttributeMask class

nikic added a comment. Pre-merge checks also report a build failure in llvm/lib/Target/AMDGPU/AMDGPURewriteOutArguments.cpp:360. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116110/new/ https://reviews.llvm.org/D116110 ___ cfe-commits maili

[PATCH] D116110: Introduce the AttributeMask class

nikic added inline comments. Comment at: llvm/include/llvm/IR/Attributes.h:992 + bool empty() const { return Attrs.none(); } + bool hasAttributes() const { return !TargetDepAttrs.empty() && Attrs.any(); } +}; Shouldn't this be `||`? Comment a

[PATCH] D116110: Introduce the AttributeMask class

nikic added a reviewer: aeubanks. nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. This looks good to me, but I'd recommend waiting a bit before landing in case there's further input, as this is a non-trivial API change. I think separating Att

[PATCH] D116110: Introduce the AttributeMask class

nikic added a comment. Looking at some of the changes here, I landed a couple of cleanup commits: - https://github.com/llvm/llvm-project/commit/3b0f5a4856fce76a6535feddd1692747b81b60cd (this should fix the build failure) - https://github.com/llvm/llvm-project/commit/e8e8bfeeb7233bde17d0a811b87

[PATCH] D116169: [JSONNodeDumper] Do not print mangled names for local variables (PR49111)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8043beb8901c: [JSONNodeDumper] Do not print mangled names for local variables (PR49111) (authored by nikic). Herald added a project: clang. Herald ad

[PATCH] D116171: [OpenMP] Fix incorrect type when casting from uintptr

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1201a0f3955b: [OpenMP] Fix incorrect type when casting from uintptr (authored by nikic). Herald added a project: clang. Herald added a subscriber: cf

[PATCH] D116214: [OpenMP] Avoid creating null pointer lvalue (NFC)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdd903173c0fb: [OpenMP] Avoid creating null pointer lvalue (NFC) (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-co

[PATCH] D93594: [X86] Pass to transform amx intrinsics to scalar operation.

nikic added a comment. It looks like this has caused a compile-time regression at `O0`: https://llvm-compile-time-tracker.com/compare.php?from=9341bcbdc93a251b632ffaa51a84452a7a4a5e4e&to=4f198b0c27b04e830a3069aaf4b39cf203eaae4a&stat=instructions The cause is probably the computation of DomTree a

[PATCH] D93594: [X86] Pass to transform amx intrinsics to scalar operation.

nikic added a comment. @yubing In this case I would recommend building `sqlite3.c` from test-suite under `perf stat` and look at the `instructions` metric. For me the command looks like this: perf stat CLANG_BINARY -w -Werror=date-time -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_

[PATCH] D97107: Replace func name with regex for update test scripts

nikic added a comment. Looks like this has broken update_analyze_test_checks.py. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97107/new/ https://reviews.llvm.org/D97107 ___ cfe-commits mailing list cfe-

[PATCH] D99121: [IR][InstCombine] IntToPtr Produces Typeless Pointer To Byte

nikic added a comment. In D99121#2650146 , @nlopes wrote: > Given that it gives a decent speedup for some important workload, and > provided it doesn't regress others [...] I think that second point is going to need some evidence (at least in the form o

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

nikic added inline comments. Comment at: llvm/lib/IR/IRBuilder.cpp:1012 // First insert it into an undef vector so we can shuffle it. Type *I32Ty = getInt32Ty(); undef vector -> poison vector Comment at: llvm/lib/IR/IRBuilder.cpp:1021

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:1073 for (unsigned i = 0; i != VWidth; ++i) { if (!DemandedElts[i]) { // If not d

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

nikic added a comment. In D93586#2472314 , @aqjune wrote: > There are 3 more patches: > > https://reviews.llvm.org/D93793 (IRBuilder's CreateVectorSplat) > https://reviews.llvm.org/D93817 (Other transformations) > https://reviews.llvm.org/D93818 (LangRef)

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. There are some polly tests that also need to be updated, but otherwise LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93793/new/ https://revi

[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

nikic added a comment. I'm somewhat skeptical about this change. The motivation seems a bit weak given the costs involved. The costs are: - Compile-time cost. Your best case estimate puts this at a 0.5% compile-time regression. This is for the case where SimplifyCFG simplify preserves DT, with

[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

nikic added a comment. In D94827#2505431 , @lebedev.ri wrote: > Wow. So much polarization here. > > In D94827#2502435 , @kuhar wrote: > >> Wow, this is fantastic. When I first started working on the domtree updater

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

nikic added a comment. As the discussion is spread out across multiple threads, do I understand correctly that the current consensus is to introduce the `-disable-noundef-analysis` flag, and explicitly add it to all the relevant tests (rather than adding it to the substitutions)? In any case,

[PATCH] D110684: [RISCV] Define _m intrinsics as builtins, instead of macros.

nikic added a comment. I noticed this a few time in the past already, but this is the worst one yet: Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=6641d29b70993bce6dbd7e0e0f1040753d38842f&to=97f0c63783f52389bd8842df205379ceade7a89d&stat=instructions Max-rss: https://llvm-

[PATCH] D112041: [InferAddressSpaces] Support assumed addrspaces from addrspace predicates.

nikic added a comment. Is it actually necessary to thread this through AssumptionCache, given how InferAddressSpaces is the only place that looks at these assumes? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112041/new/ https://reviews.llvm.org/

[PATCH] D108826: [SLP][LTO][WIP]Allow full SLP in LTO only at link time.

nikic added a comment. @ABataev The pipeline already distinguishes between pre-link and post-link optimization pipelines, see e.g. the flag that gets passed to LoopRotate to control rotation of loops with calls (https://github.com/llvm/llvm-project/blob/2f69c82cec1ae05b4fdcef4ac48f48e9e2bad32b/

<    1   2   3   4   5