[PATCH] D126689: [IR] Enable opaque pointers by default

nikic added a comment. In D126689#4654124 , @uabelho wrote: > @nikic: Thanks, nothing to do there then even if I'm surprised. > > I'm not sure but I *think* that llvm-reduce may have some problems with this. > For my out of tree target I recently saw a

[PATCH] D126689: [IR] Enable opaque pointers by default

nikic added a comment. @uabelho The call is well-defined from an LLVM IR perspective, so the verifier cannot reject it, just as it can't reject calling convention or ABI attribute mismatch. In this specific case, the call is likely undefined behavior, but that is not generally the case just

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

nikic added a comment. In D155688#4653520 , @fiigii wrote: >> The reverse transform is only done if A + B simplifies. > > Looks like`simplifyAddInst` may give add expressions, so I guess this patch > may make IC run into infinite loops. simplifyAddInst

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

nikic added a comment. In D155688#4653347 , @fiigii wrote: > How does this patch work with `visitGEPOfGEP` that does a reverse > transformation? > > // Replace: gep (gep %P, long B), long A, ... > // With:T = long A+B; gep %P, T, ... The

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

nikic added a comment. FYI this causes some compile-time regression: http://llvm-compile-time-tracker.com/compare.php?from=bc7d88faf1a595ab59952a2054418cdd0d98=af4751738db89a142a8880c782d12d4201b222a8=instructions:u About 0.7% for C++ code at `O0`. Sample for a file with >2% would be

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM We should give this a try, but I think there is a fairly large chance that this will cause regressions somewhere and a more targeted change may be necessary (e.g. only do this for

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Canonicalise ADD+GEP

nikic added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:2338-2340 +if (BinaryOperator *Idx = +dyn_cast_or_null(GEP.getOperand(1))) + if ((Idx->getOpcode() == Instruction::Add) && Idx->hasOneUse()) {

[PATCH] D155688: [PATCH] [llvm] [InstCombine] Reassociate loop invariant GEP index calculations.

nikic added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:2336 +// Try to reassociate loop invariant index calculations to enable LICM. +if (Idx && (Idx->getOpcode() == Instruction::Add)) { + Value *Ptr = GEP.getOperand(0);

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

nikic added a comment. In D86310#4648646 , @hvdijk wrote: > In D86310#4648634 , @nikic wrote: > >> I'm happy to sign off on the x86-64 part here, but I'm less sure about >> x86-32. If I understood correctly, the

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

nikic added a comment. I'm happy to sign off on the x86-64 part here, but I'm less sure about x86-32. If I understood correctly, the i128 alignment is raised there exclusively to fix the "f128 legalized to i128 libcall" case -- is there any other ABI requirement for i128 alignment on x86-32?

[PATCH] D158169: [X86] Fix i128 argument passing under SysV ABI

nikic added a comment. In D158169#4645012 , @RalfJung wrote: >> I think the CCIfSplit means it was larger than i64 to start. > > What about the case where `R9` would take the *second* half of a split i128 > (i.e., the value gets split across `R8` and

[PATCH] D158984: [Clang] Allow __declspec(noalias) to access inaccessible memory

This revision was automatically updated to reflect the committed changes. Closed by commit rG14cc7a077275: [Clang] Allow __declspec(noalias) to access inaccessible memory (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github

[PATCH] D158972: [DebugInfo] Fix incorrect dbg.declare when nrvo flag is used

This revision was automatically updated to reflect the committed changes. Closed by commit rG13a044c6993a: [DebugInfo] Fix incorrect dbg.declare when nrvo flag is used (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

nikic added a comment. FYI this resulted in some pretty wild code size swings, in particular between -10% and -15% for tramp3d-v4 (http://llvm-compile-time-tracker.com/compare.php?from=6cde64a94986165547ae5237ac7dd4bddfc9f2a7=2916b125f686115deab2ba573dcaff3847566ab9=size-text). Not sure

[PATCH] D157497: feat: Migrate isArch16Bit

nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. Herald added a subscriber: StephenFan. This change looks strictly worse in isolation, the proposal on discourse did not reach consensus, and looking at the diagram in

[PATCH] D158169: [X86] Fix i128 argument passing under SysV ABI

This revision was automatically updated to reflect the committed changes. Closed by commit rGfa1b6e6b34eb: [X86] Fix i128 argument passing under SysV ABI (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

nikic added a comment. Thanks! Can confirm that this recovers the compile-time regression. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156571/new/ https://reviews.llvm.org/D156571 ___ cfe-commits

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

nikic added a comment. To avoid a full revert, I removed the deprecations in this patch in https://github.com/llvm/llvm-project/commit/348d36f1a2c8c776ce87e038bf15fc4cabd62702. Please do not deprecate methods that have remaining in-tree users and make sure that

[PATCH] D152495: [Clang][SemaCXX] Add unused warning for variables declared in condition expressions

nikic added a comment. Please split the warning fixes off into a separate patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152495/new/ https://reviews.llvm.org/D152495 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D86993: Document Clang's expectations of the C standard library.

nikic added a comment. @aaron.ballman I wanted to check back whether the linked document is what you had in mind, or whether some more/else would be needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86993/new/ https://reviews.llvm.org/D86993

[PATCH] D155773: [llvm][MemoryBuiltins] Add alloca support to getInitialValueOfAllocation

nikic added a comment. @jmciver Thanks for the context. I would recommend you to put up the patch you have based on top of this, because it's pretty hard to tell whether this API design makes sense without seeing the use. I have some doubts about that, for two reasons: - If I understand

[PATCH] D157227: [Clang] Don't add `undef` for `operator new` under -fno-exceptions.

nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. Removing noundef makes no sense to me, because the return value is noundef even under fno-exceptions. If we remove something, it should be the nonnull attribute, because that's what's

[PATCH] D105671: [Intrinsics][ObjC] Mark objc_retain and friends as thisreturn.

nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. Herald added a subscriber: StephenFan. Please separate the change to stripPointerCasts() into a separate review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

nikic added a comment. I'm not a fan of the PGO conditional behavior here. I'd prefer to disable speculation unconditionally if that is feasible. This is basically what D153392 did. While the specific test case there was addressed in a different way, if we

[PATCH] D155991: [DWARF] Make sure file entry for artificial functions has an MD5 checksum

nikic added a comment. FYI this is a 0.5% compile-time regression on `O0` builds (https://llvm-compile-time-tracker.com/compare.php?from=69593aa5c054cec6be6b822c073ccdc63748a68d=7abb5fc618cec66841a8280d2a099a4c9c8cb91b=instructions:u). Is that expected? Repository: rG LLVM Github Monorepo

[PATCH] D86993: Document Clang's expectations of the C standard library.

nikic added a comment. Sorry for the delay, this took more time than expected. I've created an initial draft here: https://docs.google.com/document/d/1guH_HgibKrX7t9JfKGfWX2UCPyZOTLsnRfR6UleD1F8/edit?usp=sharing While writing this I remembered another bit that is somewhat relevant here: Clang

[PATCH] D155773: [llvm][MemoryBuiltins] Add alloca support to getInitialValueOfAllocation

nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. > This commit is in support of future uninitialized memory handling and adds > alloca instruction support to getInitialValueOfAllocation. This unifies > initial > memory state querying

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

nikic added a comment. In D86310#4495825 , @hvdijk wrote: > A thought occurs: in older versions of LLVM, the data layout mechanism worked > differently and permitted targets to declare that they supported multiple > different data layout strings, by

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

nikic added a comment. In D145265#4478621 , @aeubanks wrote: > @nikic could you try running this over the rust tests again? Ignoring some unrelated powerpc data layout failures, these codegen tests fail: [codegen] tests/codegen/issues/issue-45222.rs

[PATCH] D86993: Document Clang's expectations of the C standard library.

nikic added a comment. In D86993#4477080 , @aaron.ballman wrote: > In D86993#4474682 , @nikic wrote: > >>> I continue to think it's a mistake for us to document that Clang will not >>> work with a conforming C

[PATCH] D86993: Document Clang's expectations of the C standard library.

nikic added a comment. In D86993#4474392 , @rjmccall wrote: > In D86993#4474267 , @RalfJung wrote: > >> The first point is important for LLVM's own memcpy/memmove intrinsics, which >> are documented as NOPs on

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

nikic added a comment. (The patch description could use an update...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148216/new/ https://reviews.llvm.org/D148216 ___ cfe-commits mailing list

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

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. Herald added a subscriber: StephenFan. LGTM > Fixed. Does this test case exist somewhere? I couldn't find it upstream. That wasn't an existing test case. Though if you add a `--check-globals

[PATCH] D154285: [clang] Remove CGBuilderTy::CreateElementBitCast

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/D154285/new/ https://reviews.llvm.org/D154285 ___

[PATCH] D154285: [clang] Remove CGBuilderTy::CreateElementBitCast

nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. The entire change is NFC cleanup, it makes no sense to separate it. Please revert to previous patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D154285: [clang] Deprecate CGBuilderTy::CreateElementBitCast

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/CodeGen/CGBuilder.h:158 /// This method is to be deprecated. Use `Address::withElementType` instead. + [[deprecated("Use

[PATCH] D153314: [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 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153314/new/ https://reviews.llvm.org/D153314 ___

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

nikic added inline comments. Comment at: clang/lib/CodeGen/Targets/AArch64.cpp:522-523 auto *Load = CGF.Builder.CreateLoad(VAListAddr); Address Addr = Address(Load, CGF.Int8Ty, SlotSize); -return CGF.Builder.CreateElementBitCast(Addr, CGF.ConvertTypeForMem(Ty)); +

[PATCH] D153728: [llvm] Move AttributeMask to a separate header

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/D153728/new/ https://reviews.llvm.org/D153728 ___

[PATCH] D153694: [clang][CodeGen] Remove no-op EmitCastToVoidPtr (NFC)

nikic added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:1473 QualType SrcRecordTy, QualType DestTy) { auto *ClassDecl = barannikov88

[PATCH] D144970: [llvm-c] Remove bindings for creating legacy passes

nikic added a comment. In D144970#4434305 , @yamt wrote: > at least some of these stuff is used by wasm-micro-runtime. do you have a > suggestion about how to adapt it? >

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

nikic added a comment. There are some test failures. I believe there is one bug with the handling of unnamed globals. Previously we produced this: ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals --version 2 ; RUN: opt -S < %s | FileCheck

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

nikic added inline comments. Comment at: clang/lib/CodeGen/CGNonTrivialStruct.cpp:370 llvm::Value *DstArrayEnd = CGF.Builder.CreateInBoundsGEP(CGF.Int8Ty, BC.getPointer(), SizeInBytes); DstArrayEnd = CGF.Builder.CreateBitCast( Only

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

nikic added inline comments. Comment at: llvm/utils/UpdateTestChecks/common.py:1286 + if value == default_value: +continue if action.dest == 'filters': nikic wrote: > nikic wrote: > > hnrklssn wrote: > > > nikic wrote: > > > > We should also

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

nikic added inline comments. Comment at: llvm/utils/UpdateTestChecks/common.py:1286 + if value == default_value: +continue if action.dest == 'filters': nikic wrote: > hnrklssn wrote: > > nikic wrote: > > > We should also not print the `all`

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

nikic added inline comments. Comment at: llvm/utils/UpdateTestChecks/common.py:1286 + if value == default_value: +continue if action.dest == 'filters': hnrklssn wrote: > nikic wrote: > > We should also not print the `all` argument for

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM. The diff looks weird for some reason, but the downloadable `.diff` file looks fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997

[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 =

[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

[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

[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] 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( -

[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.

[PATCH] D142823: Intrinsics: Allow tablegen to mark parameters with dereferenceable

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142823/new/ https://reviews.llvm.org/D142823 ___ cfe-commits mailing list

[PATCH] D152752: [MS] Fix passing aligned records by value in some cases

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/D152752/new/ https://reviews.llvm.org/D152752 ___

[PATCH] D152226: [FunctionAttrs] Propagate some func/arg/ret attributes from caller to callsite (WIP)

nikic added inline comments. Comment at: llvm/lib/Transforms/Utils/InferCallsiteAttrs.cpp:597 + // callsite violating the constraint. + if (checkCallerDoesNotAccessMemory() && !CB->doesNotAccessMemory()) { +// Wait until we know we actually need it to do potentially

[PATCH] D152522: [NFC][SetVector] Update some usages of SetVector to SmallSetVector

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/D152522/new/ https://reviews.llvm.org/D152522 ___

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

nikic added inline comments. Comment at: clang/lib/CodeGen/CGBuilder.h:172 +auto *PtrTy = llvm::PointerType::get(Ty, Addr.getAddressSpace()); return Address(CreateBitCast(Addr.getPointer(), PtrTy, Name), Ty, Addr.getAlignment(),

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

nikic added a comment. In D152321#4403719 , @JOE1994 wrote: > Some tests that run with `-no-opaque-pointers` began failing after I updated > the revision to get rid of bitcasts. > The bitcasts are still needed if running Clang with

[PATCH] D152447: [Clang] Remove -no-opaque-pointers cc1 flag

This revision was automatically updated to reflect the committed changes. Closed by commit rG066fb7a58c5a: [Clang] Remove -no-opaque-pointers cc1 flag (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

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

nikic added inline comments. Comment at: clang/lib/CodeGen/Address.h:135 llvm::Constant *BitCast = llvm::ConstantExpr::getBitCast( -getPointer(), ElemTy->getPointerTo(getAddressSpace())); +getPointer(), llvm::PointerType::get(ElemTy, getAddressSpace()));

[PATCH] D152023: [UBSan] Consider zero input to __builtin_clz/ctz to be undefined independent of the target.

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/D152023/new/ https://reviews.llvm.org/D152023 ___

[PATCH] D150670: [InstCombine] Disable generation of fshl/fshr for rotates

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM, let's give it a try. The patch description needs an update though. Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:920 +return I;

[PATCH] D150670: [InstCombine] Disable generation of fshl/fshr for rotates

nikic added a comment. Can you please drop all wasm related tests and instead add an InstCombine test for the fsh+and pattern? It would also be good to have a test where we can fold one side to a constant, but that constant is not zero. We should then consider whether that is profitable or

[PATCH] D151701: [HIP] Add missing __hip_atomic_fetch_sub support

nikic resigned from this revision. nikic added a comment. (Looks reasonable, but is pretty far outside my area of expertise...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151701/new/ https://reviews.llvm.org/D151701

[PATCH] D150670: [WebAssembly] Disable generation of fshl/fshr for rotates

nikic added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:927-933 +if (I->getOperand(0) != I->getOperand(1)) { + APInt DemandedMaskLHS(DemandedMask.lshr(ShiftAmt)); + APInt

[PATCH] D150670: [WebAssembly] Disable generation of fshl/fshr for rotates

nikic added inline comments. Comment at: llvm/test/Transforms/InstCombine/fsh.ll:664 +; CHECK-NEXT:[[T1:%.*]] = and i32 [[A:%.*]], -65536 +; CHECK-NEXT:[[T2:%.*]] = call i32 @llvm.fshl.i32(i32 [[T1]], i32 [[T1]], i32 16) ; CHECK-NEXT:ret i32 [[T2]]

[PATCH] D150887: [clang] Convert a few tests to opaque pointers

nikic added a comment. I wanted to check whether you plan to do more opaque pointer test conversions in clang. Just to make sure we don't duplicate work... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150887/new/ https://reviews.llvm.org/D150887

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

nikic added inline comments. Comment at: clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected:12 +// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[X]], align 4 +// CHECK-NEXT:ret i32 [[TMP1]] +// hnrklssn wrote: > nikic wrote: > > hnrklssn

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

nikic added a comment. Here are two inputs that currently fold down to constants and no longer do so with this patch: https://gist.github.com/nikic/4a714ea550bf2252543570585f642af2 These need further reduction for PhaseOrdering tests, but should be a good starting point for analysis...

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

nikic added a comment. Unfortunately I'm seeing a number of Rust optimization regressions with this change. I'll try to reduce those to some PhaseOrdering tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145265/new/

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

nikic added inline comments. Comment at: clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected:12 +// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[X]], align 4 +// CHECK-NEXT:ret i32 [[TMP1]] +// hnrklssn wrote: > nikic wrote: > > nikic

[PATCH] D150670: [WebAssembly] Disable generation of fshl/fshr for rotates

nikic added a comment. In D150670#4368241 , @pmatos wrote: > In D150670#4368238 , @pmatos wrote: > >> In D150670#4352163 , @nikic wrote: >> >>> 1. Say that we prefer

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

nikic added inline comments. Comment at: clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected:12 +// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[X]], align 4 +// CHECK-NEXT:ret i32 [[TMP1]] +// nikic wrote: > hnrklssn wrote: > > hnrklssn

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. (Moving this off the review queue per above comments. There is a missing include in a unit test.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/

[PATCH] D151195: [Driver] Fix test for use of ld from devtoolset

This revision was automatically updated to reflect the committed changes. Closed by commit rG75cce50fd2d3: [Driver] Fix test for use of ld from devtoolset (NFC) (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

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

nikic added inline comments. Comment at: clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected:12 +// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[X]], align 4 +// CHECK-NEXT:ret i32 [[TMP1]] +// hnrklssn wrote: > hnrklssn wrote: > > nikic

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

nikic added a comment. The pre-merge checks have a build failure in clang-tools-extra. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 ___ cfe-commits mailing

[PATCH] D145403: [Pipeline] Don't run EarlyFPM in LTO post link

nikic added a comment. Any reason why this hasn't been landed yet? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145403/new/ https://reviews.llvm.org/D145403 ___ cfe-commits mailing list

[PATCH] D150887: [clang] Convert a few tests to opaque pointers

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/CodeGenCXX/const-init-cxx11.cpp:353 }; - // CHECK: @_ZN14VirtualMembersL13sGlobalMemoryE = internal global { i8** } { i8** getelementptr

[PATCH] D150829: [clang] Convert several tests to opaque pointers

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/CodeGenCXX/2011-12-19-init-list-ctor.cpp:22 +// CHECK: store i32 0, ptr @arr +// CHECK: call void @_ZN1AC1EPKc(ptr {{[^,]*}} getelementptr inbounds

[PATCH] D150670: [WebAssembly] Disable generation of fshl/fshr for rotates

nikic added a comment. In D150670#4352055 , @pmatos wrote: > In D150670#4351147 , @nikic wrote: > >> This doesn't looks like a wasm specific problem. You get essentially the >> same issue on any target that has

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

nikic added inline comments. Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:476 + FPM.addPass(EarlyCSEPass()); + Why the extra EarlyCSE pass in the `O1` pipeline? Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:547 +

[PATCH] D150670: [WebAssembly] Disable generation of fshl/fshr for rotates

nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. This doesn't looks like a wasm specific problem. You get essentially the same issue on any target that has a rotate instruction but no funnel shift instruction. Here are just a couple

[PATCH] D150733: [clang] Convert remaining OpenMP tests to opaque pointers

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/D150733/new/ https://reviews.llvm.org/D150733 ___

[PATCH] D150694: [clang] Convert NVPTX OpenMP tests to opaque pointers

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/D150694/new/ https://reviews.llvm.org/D150694 ___

[PATCH] D150704: [clang] Convert several smaller OpenMP tests to opaque pointers

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/D150704/new/ https://reviews.llvm.org/D150704 ___

[PATCH] D150682: [clang] Convert a few OpenMP tests to opaque pointers

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/D150682/new/ https://reviews.llvm.org/D150682 ___

[PATCH] D150680: [clang] Convert a few OpenMP tests to opaque pointers

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/D150680/new/ https://reviews.llvm.org/D150680 ___

[PATCH] D150652: [clang] Convert a few OpenMP tests to opaque pointers

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

[PATCH] D150530: [clang] Convert a few OpenMP tests to use opaque pointers

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/OpenMP/atomic_capture_codegen.cpp:868 -// CHECK: [[NEW_BF_VALUE:%.+]] = load i8, i8* [[BITCAST1]] -// CHECK: [[RES:%.+]] = cmpxchg i8*

[PATCH] D150520: [clang] Convert a few tests to opaque pointers

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

[PATCH] D150520: [clang] Convert a few tests to opaque pointers

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/D150520/new/ https://reviews.llvm.org/D150520 ___

[PATCH] D145788: [CodeGen] Only consider innermost cast for !heapallocsite

This revision was automatically updated to reflect the committed changes. Closed by commit rGcac4d7ff4652: [CodeGen] Only consider innermost cast for !heapallocsite (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D149641: [docs] Hide collaboration and include graphs in doxygen docs

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. Herald added a subscriber: StephenFan. LGTM, only the inheritance graph is useful, which this preserves if I understand correctly (CLASS_GRAPH is still YES). Repository: rG LLVM Github

[PATCH] D140538: [Clang][CodeGen] Use poison instead of undef for dummy values in CGExpr [NFC]

nikic added a comment. The demangling test shouldn't be modified at all. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140538/new/ https://reviews.llvm.org/D140538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

nikic added a comment. In D143624#4316357 , @aeubanks wrote: > In D143624#4315508 , @nikic wrote: > >> In D143624#4315468 , @dmgreen >> wrote: >> >>> It looks like there

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

nikic added a comment. In D143624#4315468 , @dmgreen wrote: > It looks like there is quite a lot more optimization that happens to the > function being always-inlined (__SSAT) before this change. Through multiple > rounds of instcombine, almost to the

[PATCH] D149210: [IR] Change shufflevector undef mask to poison

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

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

nikic added inline comments. Comment at: clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected:12 +// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[X]], align 4 +// CHECK-NEXT:ret i32 [[TMP1]] +// hnrklssn wrote: > delcypher wrote: > >

  1   2   3   4   5   >