[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

nikic added a comment. In D125683#3815348 , @ldionne wrote: > In D125683#3811492 , @nikic wrote: > >> I have some concerns about this change: >> >> - It makes it fundamentally impossible to link against a static

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

nikic added a comment. In D20401#3824569 , @nickdesaulniers wrote: > But I wouldn't be surprised if other Linux distro's like RHEL bootstrap their > clang distribution via GCC. @tstellar or @serge-sans-paille or @nikic might > know. We did get a curio

[PATCH] D135259: [clang] Remove CLANG_ENABLE_OPAQUE_POINTERS cmake option

This revision was automatically updated to reflect the committed changes. Closed by commit rGd785a8eaa25d: [clang] Remove CLANG_ENABLE_OPAQUE_POINTERS cmake option (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CH

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

nikic added a comment. FYI this caused a noticeable compile-time regression (about 0.4% geomean at `-O0`): http://llvm-compile-time-tracker.com/compare.php?from=92233159035d1b50face95d886901cf99035bd99&to=371883f46dc23f8464cbf578e2d12a4f92e61917&stat=instructions Repository: rG LLVM Github M

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

nikic added a comment. In D135118#3851050 , @akyrtzi wrote: > @nikic, it seems to have recovered > (http://llvm-compile-time-tracker.com/compare.php?from=c49cde6467f9bf200640db763152a9dc7f009520&to=0456acbfb942f127359a8defd1b4f1f44420df3e&stat=instructio

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. The default switch has happened, so unblocking this. Comment at: clang/test/CodeGenOpenCL/overload.cl:23 generic int *local *genloc; generic int *global *genglob; + // C

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

nikic added subscribers: arsenm, nikic. nikic added a comment. Checking back here, have you made any progress on reducing the issue? cc @arsenm for awareness Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135269/new/ https://reviews.llvm.org/D13526

[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

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/D139881/new/ https://reviews.llvm.org/D139881 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D141349: Remove the ThreadLocal template from LLVM.

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. Herald added a subscriber: StephenFan. LGTM > As a precaution, use LLVM_THREAD_LOCAL which provides even greater > backwards compatibility, allowing this to function even pre-C++11 > versions of

[PATCH] D140699: [OptTable] Make ValuesCode initialisation of Options constexpr

nikic added inline comments. Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:268 +if (!isa(R.getValueInit("ValuesCode"))) { + assert(!isa(R.getValueInit("Values")) && + "Cannot choose between Values and ValuesCode"); Isn't this assert the

[PATCH] D140800: [OptTable] Precompute OptTable prefixes union table through tablegen

nikic added a comment. Just to check, this isn't going to cause some warning spew about all those OptTable implementations being non-final? Comment at: llvm/include/llvm/Option/OptTable.h:256 +/// Specialization of +class GenericOptTable : public OptTable {

[PATCH] D141092: Optionally install clang-tblgen to aid cross-compiling

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/D141092/new/ https://reviews.llvm.org/D141092 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D135780: [IR] Switch everything to use memory attribute

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG304f1d59ca41: [IR] Switch everything to use memory attribute (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commi

[PATCH] D137475: Explicitly initialize opaque pointer mode when -fthinlto-index is used

nikic added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:1112 +// mixing opaque pointers and typed pointers bitcode files. +VMContext->setOpaquePointers(CI.getCodeGenOpts().OpaquePointers); + I think it would be fine to always do this,

[PATCH] D137475: Explicitly initialize opaque pointer mode in CodeGenAction

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

[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

nikic added a comment. I'd like to object to enabling this via the frontend. This means that non-clang frontends will now use a non-default configuration that is not extensively tested by upstream anymore. If you don't want to change tests, you can adjust the RUN lines to explicitly disable gl

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

nikic added a comment. @TimNN Do you have an IR sample for this? DAE is supposed to strip UB-implying attributes when converting arguments to poison here: https://github.com/llvm/llvm-project/blob/cbe5b2dd914b7ee47bb4d0f67af154a40be4d208/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp#LL291C

[PATCH] D142373: [Utils] Add --full-function-signature to update_cc_test_checks.py to match return type as well as args

nikic added a comment. Please give me a day to propose another alternative... Seeing D142452 we now have three different patches that would like to change UTC output, so I really think we need that `--version` flag. Repository: rG LLVM Github Monorepo CHA

[PATCH] D142373: [Utils] Add --full-function-signature to update_cc_test_checks.py to match return type as well as args

nikic added a comment. D142473 would be my proposal to fix the test churn problem going forward. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142373/new/ https://reviews.llvm.org/D142373 ___

[PATCH] D142473: [UTC] Add --version argument

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5375b638be87: [UTC] Add --version argument (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository:

[PATCH] D142872: Honor the fwrapv option when generating the inbounds GEP .

nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. CreateConstArrayGEP currently always creates an inbounds GEP. You need to modify it make inbounds option / use a different method, not supress it in the constant folding infrastructure

[PATCH] D142872: Honor the fwrapv option when generating the inbounds GEP .

nikic added a comment. You also shouldn't need to introduce any new handling for the fwrapv option. I'm not sure how exactly it is currently threaded through, but there must be existing handling for it already. For example, there is isSignedOverflowDefined() on LangOptions. Repository: rG L

[PATCH] D142872: Honor the fwrapv option when generating the inbounds GEP .

nikic added a comment. @umesh.kalappa0 I see. That part is actually a bug in LLVM. I'm working on a fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142872/new/ https://reviews.llvm.org/D142872 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D142872: Honor the fwrapv option when generating the inbounds GEP .

nikic added a comment. @umesh.kalappa0 This issue should be fixed by https://github.com/llvm/llvm-project/commit/5f01a626dd0615df49773d419c75aeb33666ee83. Can you please give it another try? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142872/new/ https://reviews.llvm.org/D142872 __

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

nikic added a comment. FYI this causes a minor compile-time regression (around 0.35% on 7zip at `O0`): http://llvm-compile-time-tracker.com/compare.php?from=cd173cbd7cca69c29df42cd4b42e60433435c29b&to=d708a186b6a9b050d09558163dd353d9f738c82d&stat=instructions%3Au Just wanted to check whether tha

[PATCH] D142826: [Clang] Add -Wtype-limits to -Wextra for GCC compatibility

nikic added a comment. @aaron.ballman It works with the URL as well. The URL is preferred, because that way people don't have to reconstruct it when seeing the issue reference on phabricator, where it does not get auto-linked. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142872: Honor the fwrapv option when generating the inbounds GEP .

nikic added a comment. I've found another bug in LLVM, fixed with https://github.com/llvm/llvm-project/commit/78f88082de3627ea04501c83a08f52cf1e60b4f7. After that change, the test case from https://github.com/llvm/llvm-project/issues/59580 appears to be handled correctly, it contains this in t

[PATCH] D141092: Optionally install clang-tblgen to aid cross-compiling

nikic added a comment. Can you please share the `Name ` to use for the commit? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141092/new/ https://reviews.llvm.org/D141092 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D141092: Optionally install clang-tblgen to aid cross-compiling

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG706881825b24: [cmake] Optionally install clang-tblgen to aid cross-compiling (authored by chewi, committed by nikic). Repository: rG LLVM Github M

[PATCH] D140699: [OptTable] Make ValuesCode initialisation of Options constexpr

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/D140699/new/ https://reviews.llvm.org/D140699 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D141494: [Clang] Emit noundef metadata next to range metadata

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG02856565ac12: [Clang] Emit noundef metadata next to range metadata (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe

[PATCH] D140800: [OptTable] Precompute OptTable prefixes union table through tablegen

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM In D140800#4044983 , @serge-sans-paille wrote: > In D140800#4043723 , @nikic wrote: > >> Just to check,

[PATCH] D141899: [IR][X86] Remove X86AMX type in LLVM IR instead of target extension

nikic added a comment. From a very cursory look, this looks pretty nice. It looks like we pretty much get rid of AMX-specific knowledge in the middle-end entirely, which is the point of target extension types. Regarding bitcasts, it's worth mentioning that there was some recent discussion abou

[PATCH] D141899: [IR][X86] Remove X86AMX type in LLVM IR instead of target extension

nikic added a comment. I think it would also be fine to retain the Type::isX86_AMXTy(), at least for now. That would reduce this patch to mostly the representation change, without the change to check for the type in a different way. Comment at: llvm/lib/IR/Core.cpp:655 -LLVMT

[PATCH] D142026: Optimize OptTable::findNearest implementation and usage

nikic added inline comments. Comment at: clang/lib/Driver/Driver.cpp:2475 if (getOpts().findNearest(Value, Nearest, IncludedFlagsBitmask, - ExcludedFlagsBitmask) <= 1) { + ExcludedFlagsBitmask, 0, 1) <= 1) { Di

[PATCH] D142026: Optimize OptTable::findNearest implementation and usage

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/D142026/new/ https://reviews.llvm.org/D142026 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D141899: [IR][X86] Remove X86AMX type in LLVM IR instead of target extension

nikic added a comment. In D141899#4061237 , @zixuan-wu wrote: > With considering > https://llvm.org/docs/DeveloperPolicy.html#ir-backwards-compatibility I think > we need make consensus to choose one option from following 2 options. > > 1. Remove X86amx

[PATCH] D142024: [clang] Optimize clang::Builtin::Info density

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM. The task-clock improvement is likely noise, but there's definitely a code size / max-rss win here. Comment at: clang/include/clang/Basic/BuiltinHeaders.def:9 +// +// Thi

[PATCH] D142181: [OptTable] Store llvm::cl::Option into a DenseMap instead of a StringMap

nikic added a comment. FAILED: tools/mlir/test/lib/Dialect/Linalg/CMakeFiles/MLIRLinalgTestPasses.dir/TestLinalgHoisting.cpp.o CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++ -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -DMLIR_CUDA_CONVERSIONS_ENABLED=1 -DMLIR_INCLUDE_TESTS -DMLI

[PATCH] D142026: Optimize OptTable::findNearest implementation and usage

nikic added a comment. FYI I just noticed some i686 failures that look something like this: /builddir/build/BUILD/llvm-16.0.0.src/redhat-linux-build/unittests/Option/./OptionTests --gtest_filter=OptTableTest/0.FindNearest -- /builddir/build/BUILD/llvm-16.0.0.src/unittests/Option/OptionPa

[PATCH] D142181: [OptTable] Store llvm::cl::Option into a DenseMap instead of a StringMap

nikic added a comment. In D142181#4070868 , @serge-sans-paille wrote: > @nikic : I thought I fixed that with e8a163dc03e6913360beb305620104ba129c081c > ... is > it included in your b

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

nikic added inline comments. Comment at: clang/cmake/modules/AddClang.cmake:178 # Always generate install targets - llvm_install_symlink(${name} ${dest} ALWAYS_GENERATE) + llvm_install_symlink(CLANG name} ${dest} ALWAYS_GENERATE) endmacro() Yeah, there's a

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

nikic added a comment. In D125291#3548671 , @jyknight wrote: > Anyhow -- I think the prototype I'm fiddling with is also along the path to > the ideal long-term state, but pushing it beyond a prototype seems like it'll > be a pain in the ass due to the

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

nikic added a reviewer: opaque-pointers. nikic added a comment. Could you please add some context directly to the patch description that explains why this is necessary? Being unfamiliar with SPRIV, I don't really get what is significant about the code example. Repository: rG LLVM Github Mono

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

nikic added a comment. In D127579#3585516 , @beanz wrote: > @nikic the most important thing you need to know about SPIR-V is that it is a > virtual ISA based on LLVM IR. The ISA itself encodes types for pointers just > like LLVM IR would. Okay ... I gu

[PATCH] D126061: [clang] Reject non-declaration C++11 attributes on declarations

nikic added a comment. FYI this change had a measurable effect on compile-time (http://llvm-compile-time-tracker.com/compare.php?from=7acc88be0312c721bc082ed9934e381d297f4707&to=8c7b64b5ae2a09027c38db969a04fc9ddd0cd6bb&stat=instructions), about 0.5% regression for `O0` builds. Not sure if that's

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

nikic added a comment. @Anastasia Thanks, that does sound like a legitimate reason to include the information. I want to double check though, does linking the modules actually fail if the functions have signatures that differ only by pointer types? At least for normal LLVM IR this would work fi

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

nikic added a comment. In D127579#3588626 , @Anastasia wrote: > In D127579#3586092 , @nikic wrote: > >> @Anastasia Thanks, that does sound like a legitimate reason to include the >> information. I want to double

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

nikic added a comment. In D127579#3598306 , @Anastasia wrote: > In D127579#3595461 , @nikic wrote: > >> In D127579#3588626 , @Anastasia >> wrote: >> >>> In D127579#358609

[PATCH] D128439: [Clang][WIP] Don't call distributeTypeAttrsFromDeclarator() on empty list.

nikic added a comment. Not seeing a statistically significant impact from this change: http://llvm-compile-time-tracker.com/compare.php?from=8b6f69a4da5baaf3748798a84dd16a2481b7ca7f&to=797ba50f5fd88017925fe765427b1f5f136c3310&stat=instructions Maybe it's an improvement at the 0.05% level, but it

[PATCH] D128097: [Clang] Fix compile time regression caused by D126061.

nikic added a comment. @mboehme Cycle counts are very noisy, and it's pretty much impossible to determine whether they changed by looking at a single commit, unless the differences are huge (like 10%). In this case, the commit got "lucky" and the next commit goes back to the previous level. Thi

[PATCH] D128439: [Clang][WIP] Don't call distributeTypeAttrsFromDeclarator() on empty list.

nikic added a comment. In D128439#3607289 , @mboehme wrote: > Just a question for my understanding before I abandon the change: Shouldn't > the "instructions" count be relatively resistant to noise? (I was assuming > this is based on performance counter

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

nikic added a comment. Looks reasonable to me, but probably @MaskRay should review. We could add a `-fno-opaque-pointers` driver option that covers both the compiler backend and LTO, but we can probably get away without it as well. Comment at: lld/ELF/Options.td:311 + "Use o

[PATCH] D126308: cmake: use llvm dir variables for clang/utils/hmaptool

nikic added a comment. It looks like this change breaks the standalone build for clang. From what I can see, while LLVM_TOOLS_BINARY_DIR and LLVM_TOOLS_INSTALL_DIR are exported in llvm/cmake/modules/LLVMConfig.cmake.in, LLVM_UTILS_INSTALL_DIR is not, resulting in an "install PROGRAMS given no D

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

nikic accepted this revision. nikic added a comment. LGTM Comment at: clang/test/CodeGen/thinlto-inline-asm2.c:5 +// RUN: %clang_cc1 -opaque-pointers -triple x86_64-unknown-linux-gnu -flto=thin -emit-llvm-bc %t/a.c -o %t/a.bc +// RUN: %clang_cc1 -opaque-pointers -triple x86_64

[PATCH] D126308: cmake: use llvm dir variables for clang/utils/hmaptool

nikic added a comment. In D126308#3546235 , @mizvekov wrote: > In D126308#3545937 , @Ericson2314 > wrote: > >> There is a lot of cruft behind the signs. I would just use >> `LLVM_TOOLS_INSTALL_DIR` then which is

[PATCH] D126340: [clang][AIX] add option -mdefault-visibility-export-mapping

nikic added a comment. It looks like this causes a major compile-time regression in the clang frontend: http://llvm-compile-time-tracker.com/compare.php?from=6232a8f3d61e5856c17e7b314385e9ea8068cdc1&to=8c8a2679a20f621994fa904bcfc68775e7345edc&stat=instructions For example, tramp3d-v4 is up 5% i

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

nikic added a comment. @uabelho Here's a slightly cleaned up test case that does not use opaque pointers: target triple = "x86_64-unknown-linux-gnu" define void @test(i1 %cond, <1 x i16>* %p) { br label %loop loop: %v = load <1 x i16>, <1 x i16>* %p, align 2 %ins = insert

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

nikic added a comment. I believe https://reviews.llvm.org/D126886 should fix the DAGCombiner issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126689/new/ https://reviews.llvm.org/D126689 ___ cfe-comm

[PATCH] D109977: LLVM Driver Multicall tool

nikic added inline comments. Comment at: llvm/cmake/modules/AddLLVM.cmake:890 +configure_file( + ${LLVM_MAIN_SRC_DIR}/cmake/driver-template.cpp.in + ${CMAKE_CURRENT_BINARY_DIR}/${name}-driver.cpp) chapuni wrote: > Do you have a plan to export the te

[PATCH] D109977: LLVM Driver Multicall tool

nikic added inline comments. Comment at: llvm/cmake/modules/AddLLVM.cmake:890 +configure_file( + ${LLVM_MAIN_SRC_DIR}/cmake/driver-template.cpp.in + ${CMAKE_CURRENT_BINARY_DIR}/${name}-driver.cpp) nikic wrote: > chapuni wrote: > > Do you have a plan

[PATCH] D109977: LLVM Driver Multicall tool

nikic added inline comments. Comment at: llvm/cmake/modules/AddLLVM.cmake:890 +configure_file( + ${LLVM_MAIN_SRC_DIR}/cmake/driver-template.cpp.in + ${CMAKE_CURRENT_BINARY_DIR}/${name}-driver.cpp) nikic wrote: > nikic wrote: > > chapuni wrote: > > >

[PATCH] D126308: cmake: use llvm dir variables for clang/utils/hmaptool

nikic added a comment. In D126308#3568696 , @mizvekov wrote: > @nikic ping, can you confirm this current patch works for you? Sorry, missed the update. Just tried this in a local build and it seems to work fine! Repository: rG LLVM Github Monorepo

[PATCH] D127471: [Coroutines] Convert coroutine.presplit to enum attr

nikic added a comment. You probably want to add this to `llvm::UpgradeAttributes` as well, to maintain support for old bitcode. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127471/new/ https://reviews.llvm.org/D127471 ___

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

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

[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? Is

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

[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] 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] 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 loop-inv

[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] 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] 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&to=7abb5fc618cec66841a8280d2a099a4c9c8cb91b&stat=instructions:u). Is that expected? Repository: rG LLVM Github Mon

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

[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] 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 https://lists.

[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 `-Werror=deprecated-declarations`

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

[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: https://reviews.llvm.org

[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 https://discourse

[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&to=af4751738db89a142a8880c782d12d4201b222a8&stat=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 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 reverse

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

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

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

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

[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] 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 `R9

[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 `Address::withElementT

[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 https://reviews

[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 ___ c

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

[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 cfe-commits@l

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

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

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

[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&to=2916b125f686115deab2ba573dcaff3847566ab9&stat=size-text). Not sure

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

<    1   2   3   4   5   >