[PATCH] D123957: Update the developer policy to mention release notes

2022-04-19 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added inline comments. This revision is now accepted and ready to land. Comment at: llvm/docs/DeveloperPolicy.rst:195 +* Adding, removing, or regrouping a diagnostic. +* Fixing a bug (please link to the issue fixed in the bug database). +* Addi

[PATCH] D124063: [LegacyPM] Rename and deprecate populateModulePassManager

2022-04-20 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Please do not change names in the FFI API at least. It's okay to drop it entirely, but renaming is unnecessarily hostile for FFI APIs. Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:95 +#endif + Builder.populateModulePassManagerLegacy(

[PATCH] D124063: [LegacyPM] Rename and deprecate populateModulePassManager

2022-04-20 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D124063#3461171 , @MaskRay wrote: > In D124063#3461133 , @nikic wrote: > >> Please do not change names in the FFI API at least. It's okay to drop it >> entirely, but renaming is unnecess

[PATCH] D118792: [lld][clang][cmake] Clean up a few things

2022-02-03 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. Not very familiar with cmake, but this looks reasonable to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118792/new/ https://reviews.llvm.org/

[PATCH] D110445: [NFC] Avoid some AttrBuilder redundant initialization

2022-02-04 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. This one can probably be abandoned after the AttrMask/AttrBuilder changes that have been made? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110445/new/ https://reviews.llvm.org/D110445 _

[PATCH] D118894: [OpenCL] Mark kernel arguments as ABI aligned

2022-02-08 Thread Nikita Popov via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG18834dca2d78: [OpenCL] Mark kernel arguments as ABI aligned (authored by nikic). Herald added subscribers: cfe-commits, ldrumm. Herald added a projec

[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

2022-02-09 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. It looks like this breaks expensive checks builds due to machine verifier errors: https://lab.llvm.org/buildbot/#/builders/16/builds/24103 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110869/new/ https://reviews.llvm.org/D1

[PATCH] D123300: [Clang] Enable opaque pointers by default

2022-04-22 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D123300#3467278 , @markus wrote: > We have run into a slight performance degrading issue with our downstream > target. The situation is that we relay on the "consthoist" pass with option > "-consthoist-gep=1" to factor out the

[PATCH] D123300: [Clang] Enable opaque pointers by default

2022-04-22 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D123300#3467615 , @yurai007 wrote: > Just one more thing regarding this: > > In D123300#3467165 , @yurai007 > wrote: > >> Hi, unfortunately for some reason it doesn't play well with coro

[PATCH] D124063: [LegacyPM] Rename and deprecate populateModulePassManager

2022-04-25 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. @jberdine See https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm-c/Transforms/PassBuilder.h for C bindings for the new pass manager. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124063/new/ https://reviews.ll

[PATCH] D123300: [Clang] Enable opaque pointers by default

2022-04-26 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. @yurai007 I've put up https://reviews.llvm.org/D124459 to fix this optimization failure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123300/new/ https://reviews.llvm.org/D123300 ___

[PATCH] D124504: Remove --no-opaque-pointers in test/cxx2a-thread-local-constinit.cpp

2022-04-27 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a reviewer: opaque-pointers. nikic added a comment. How does it block your work? Tests currently still use `-no-opaque-pointers` to avoid breaking this mode. I think it's generally okay to drop the option, but I'm a bit unclear about the motivation in this case -- do you want to make

[PATCH] D124504: Remove --no-opaque-pointers in test/cxx2a-thread-local-constinit.cpp

2022-04-27 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D124504#3476562 , @ChuanqiXu wrote: > In D124504#3476537 , @nikic wrote: > >> How does it block your work? Tests currently still use `-no-opaque-pointers` >> to avoid breaking this mode.

[PATCH] D124563: Drop '* text=auto' from .gitattributes and normalize

2022-04-28 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. This change seems to have broken switching to a branch before the commit completely. The only way I was able to recover my git checkout is to comment out the `* text=auto` line and then run `git reset --hard`, otherwise the crlf.cpp files always show up as changed. Repo

[PATCH] D124159: [SimplifyCFG] Thread branches on same condition in more cases (PR54980)

2022-04-29 Thread Nikita Popov via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4e545bdb355a: [SimplifyCFG] Thread branches on same condition in more cases (PR54980) (authored by nikic). Herald added a project: clang. Herald adde

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-13 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. FYI this change had a noticeable compile-time impact (http://llvm-compile-time-tracker.com/compare.php?from=ee88c0cf09969ba44307068797e12533b94768a6&to=bdc6974f92304f4ed542241b9b89ba58ba6b20aa&stat=instructions), is that expected? Largest regressions are in kimwitu++, wher

[PATCH] D129288: [IR] Don't use blockaddresses as callbr arguments

2022-07-15 Thread Nikita Popov via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2a721374aef3: [IR] Don't use blockaddresses as callbr arguments (authored by nikic). Herald added a reviewer: bollu. Herald added a project: clang. H

[PATCH] D129954: [CodeGen][inlineasm] assume the flag output of inline asm is boolean value

2022-07-17 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Hm, might it be better to teach LLVM about this (in some ConstantRange/KnownBits logic)? This doesn't really seem like something the frontend should know about and encode, as that means each frontend has to add this annotation by itself. Repository: rG LLVM Github Mon

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-18 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Given that LLVM 15 branches off in one week, maybe it would be better to wait for that before relanding the change, as it seems to have significant impact on plugins? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/

[PATCH] D130268: [WIP] Add SmallVector constructor to allow creation of SmallVector from ArrayRef of items convertible to type T

2022-07-21 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:504 llvm::StructType *STy = llvm::ConstantStruct::getTypeForElements( - CGM.getLLVMContext(), Packed ? PackedElems : UnpackedElems, Packed); + CGM.getLLVMContext(), Packed ? PackedElems : t

[PATCH] D130268: [WIP] Add SmallVector constructor to allow creation of SmallVector from ArrayRef of items convertible to type T

2022-07-21 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:504 llvm::StructType *STy = llvm::ConstantStruct::getTypeForElements( - CGM.getLLVMContext(), Packed ? PackedElems : UnpackedElems, Packed); + CGM.getLLVMContext(), Packed ? PackedElems : t

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

2022-07-21 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added a comment. Still LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117977/new/ https://reviews.llvm.org/D117977 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D120301: clang-tools-extra: Make test dependency on LLVMHello optional

2022-05-09 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments. Comment at: clang-tools-extra/test/CMakeLists.txt:107 + if (TARGET LLVMHello) +list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule) + endif() Shouldn't this be LLVMHello? Repository: rG LLVM Github Monorepo CHAN

[PATCH] D125378: [Attribute] Introduce shuffle attribute to be used for __shfl_sync like cross-lane APIs

2022-05-11 Thread Nikita Popov via Phabricator via cfe-commits
nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. Please specify the semantics of the new LLVM attribute in LangRef -- though I don't really understand why you need an LLVM-side attribute at all. CHANGES SINCE LAST ACTION https://r

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

2022-05-20 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments. Comment at: llvm/include/llvm/LTO/Config.h:182 + /// unless already set by the `-opaque-pointers` commandline option. + bool OpaquePointers = false; + The default should be true here and for the LLD/LLVMgold flags, to match the Cla

[PATCH] D126191: [Clang][CodeGen] Fix the cmse-clear-return.c test.

2022-05-23 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. Not familiar with this test, but the change looks reasonable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126191/new/ https://reviews.llvm.org/D

[PATCH] D87603: [X86] Update SSE/AVX integer MINMAX intrinsics to emit llvm.smax.* etc. (PR46851)

2020-09-14 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM. I think we have enough to start using them for vector intrinsics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87603/new/ https://reviews.l

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

2020-09-17 Thread Nikita Popov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG05d4c4ebc2fb: [InstCombine] Canonicalize SPF_ABS to abs intrinc (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

2020-09-17 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D87188#2277728 , @spatel wrote: > LGTM - I think we should give this a try as-is (with the one-use check still > there), see if anything regresses, then ease/remove the use check as a > follow-on. Okay, let's do that. > As not

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

2020-09-18 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D87188#2281093 , @mstorsjo wrote: > This broke a few tests for me (generating code that now gives the fail result > at runtime). > > I'm not entirely sure which bit is the culprit, but the difference in output > (that breaks tes

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

2020-08-21 Thread Nikita Popov via Phabricator via cfe-commits
nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. This change adds three PDT calculations to the standard pipeline. Please try to avoid the PDT calculations if PGO is not used, possibly by using LazyBPI. CHANGES SINCE LAST ACTION h

[PATCH] D86488: [X86] Default to -mtune=generic unless -march is passed to the driver. Add TuneCPU to the AST serialization

2020-08-27 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. This change has a 0.3% compile-time regression (https://llvm-compile-time-tracker.com/compare.php?from=0c55889d809027136048a0d144209a2bc282e7fc&to=71f3169e1baeff262583b35ef88f8fb6df7be85e&stat=instructions) and a 0.3% max-rss regression (https://llvm-compile-time-tracker.

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

2020-08-27 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Current compile-time numbers show a 1% geomean regression: https://llvm-compile-time-tracker.com/compare.php?from=d7c119d89c5f6d0789cfd0a139c80e23912c0bb0&to=127615d90c7b4424ec83f5a8ab4256d08f7a8362&stat=instructions I've left a comment inline with a possible cause. ===

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

2020-08-28 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Herald added a subscriber: danielkiss. New compile-time numbers: https://llvm-compile-time-tracker.com/compare.php?from=d7c119d89c5f6d0789cfd0a139c80e23912c0bb0&to=e0a1a6cac1b982023f8ceba8285d1ee7bc96bd32&stat=instructions The regression is now reduced to 0.2%. I assume th

[PATCH] D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults

2020-08-29 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. Thanks, looks good now :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 ___ cfe-commits mailing l

[PATCH] D85778: More accurately compute the ranges of possible values for +, -, *, &, %.

2020-09-01 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Just for the record, the additional analysis has a measurable compile-time impact (0.3% at O0): https://llvm-compile-time-tracker.com/compare.php?from=e7f53044e7263cdbbb4fed9abf086b88ba486bba&to=cff6dda604cb0548bef5e5951dd1e74536110588&stat=instructions Repository: rG

[PATCH] D87101: [X86] Update SSE/AVX ABS intrinsics to emit llvm.abs.* (PR46851)

2020-09-03 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. I think our abs intrinsic support is already sufficient to start canonicalizing to the intrinsic variant (the same is not true for min/max intrinsics). We might want to make the InstCombine change before this one, to make sure we don't lose CSE opportunities between the i

[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

2020-09-09 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added a comment. LGTM as well, per llvm-dev discussion. I think you've done all the due diligence we can expect. Do you plan to also work on MSSA-based MemCpyOpt? I expect that one will have a large positive impact on Rust code, where lack of cross-BB memcpy

[PATCH] D93002: [NPM] Support -fmerge-functions

2020-12-10 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments. Comment at: llvm/lib/Passes/PassBuilder.cpp:1776 + if (PTO.MergeFunctions) +MPM.addPass(MergeFunctionsPass()); + In the legacy PM this is placed after the AlwaysInlinerPass rather than before it: https://github.com/llvm/llvm-pr

[PATCH] D93002: [NPM] Support -fmerge-functions

2020-12-10 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93002/new/ https://reviews.llvm.org/D93002 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

2020-12-11 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. This also crashes test-suite, so I've reverted the change for now. (Stack trace for tramp3d-v4 crash: https://llvm-compile-time-tracker.com/show_error.php?commit=7b3470baf8bab1919e3ad4c18e2b776c1f7be2d5) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

2020-12-20 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D86844#2465027 , @jdoerfert wrote: > @nikic Is this OK or do you want it split? After looking again, I assume this can't really be split, because prior to this change we would never delete a loop without an exit block, so the co

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

2020-11-12 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D87188#2385619 , @spatel wrote: > D90554 / f7eac51b9b > > I think that change makes this patch ready to try again. If we do s

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

2020-11-13 Thread Nikita Popov via Phabricator via cfe-commits
nikic updated this revision to Diff 305232. nikic added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87188/new/ https://reviews.llvm.org/D87188 Files: clang/test/CodeGen/builtins-wasm.c llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp llvm/test/Transforms

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

2020-10-04 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. I'll just say this LGTM as it establishes parity with what NewPM has been doing for a while already. Reviewers, in the future, please reject any patches that only change the NewPM pipeline or o

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

2020-10-05 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. This ended up having a rather large impact... Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=567462b48eba1c2d286ce97117994463f4535d2e&to=e00f189d392dd9bf95f6a98f05f2d341d06cd65c&stat=instructions Code size: https://llvm-compile-time-tracker.com/compa

[PATCH] D88979: [InstCombine] combineLoadToOperationType(): don't fold int<->ptr cast into load

2020-10-11 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Looking through other uses of isNoopCast(), I don't think it makes sense to push this change into it, as many other usages do need it to work with ptrtoint/inttoptr (some of them using it

[PATCH] D88979: [InstCombine] combineLoadToOperationType(): don't fold int<->ptr cast into load

2020-10-11 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:560 // Fold away bit casts of the loaded value by loading the desired type. - // We can do this for BitCastInsts as well as casts from and to pointer types, - // as long

[PATCH] D88979: [InstCombine] combineLoadToOperationType(): don't fold int<->ptr cast into load

2020-10-11 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D88979#2323940 , @lebedev.ri wrote: > In D88979#2323935 , @nikic wrote: > >> Looking through other uses of isNoopCast(), I don't think it makes sense to >> push this change into it, as man

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-29 Thread Nikita Popov via Phabricator via cfe-commits
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/D92270/new/ https://reviews.llvm.org/D92270 ___ cfe

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

2020-11-30 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments. Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:661 + } +} } These fixes look unrelated. Is it possible to test them separately? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D92072: [CMake][NewPM] Move ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER into llvm/

2020-12-01 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. It looks like this change has also enabled the new pass manager by default. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92072/new/ https://reviews.llvm.org/D92072 ___ cfe-commits

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

2020-12-06 Thread Nikita Popov via Phabricator via cfe-commits
nikic abandoned this revision. nikic added a comment. This needs someone with access to AArch64 hardware to look into https://reviews.llvm.org/D87188#2281093 to make progress. I don't have any AArch64 hardware, and judging by the time it takes to build cmake on an AArch64 machine in the GCC com

[PATCH] D89394: clang/Basic: Stop using SourceManager::getBuffer, NFC

2020-10-18 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. FYI this showed up as a 1% compile-time regression in terms of instructions retired for `O0-g` builds: https://llvm-compile-time-tracker.com/compare.php?from=32a4ad3b6ce6028a371b028cf06fa5feff9534bf&to=54c1bcab90102481fe43b73f8547d47446ba2163&stat=instructions Looking at

[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

2020-10-22 Thread Nikita Popov via Phabricator via cfe-commits
nikic reopened this revision. nikic added a comment. This revision is now accepted and ready to land. Reopening this so we don't forget... I believe @spatel is working on the cost modelling. I did not have much luck tracking down the miscompile, at least did not spot anything incriminating in t

[PATCH] D89158: [NewPM] Run all EP callbacks under -O0

2020-10-26 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1439 if (CodeGenOpts.OptimizationLevel == 0) { + PB.runRegisteredEPCallbacks(MPM, Level, CodeGenOpts.DebugPassManager); + It should be possible to simplify this function a lot no

[PATCH] D89158: [NewPM] Run all EP callbacks under -O0

2020-10-28 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. If you are not invoking all the needed callbacks in invokeRegisteredO0EPCallbacks(), I would suggest to instead provide a set of methods like invokePipelineStartEPCallbacks(), invokeOptimizerLastEPCallbacks() that allow the consumer to chose which callbacks to invoke. (Ju

[PATCH] D103491: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC)

2021-06-01 Thread Nikita Popov via Phabricator via cfe-commits
nikic created this revision. nikic added reviewers: lattner, RKSimon. Herald added subscribers: foad, dcaballe, cota, teijeong, dexonsmith, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, kerbowa, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, csigg, antiagainst, sha

[PATCH] D103491: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC)

2021-06-02 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments. Comment at: llvm/include/llvm/ADT/ArrayRef.h:595-600 + if (RHS.data() == getEmptyKey().data()) +return LHS.data() == getEmptyKey().data(); + if (RHS.data() == getTombstoneKey().data()) +return LHS.data() == getTombstoneKey().

[PATCH] D94633: [FunctionAttrs] Infer willreturn for functions without loops

2021-01-21 Thread Nikita Popov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG65fd034b95d6: [FunctionAttrs] Infer willreturn for functions without loops (authored by nikic). Herald added subscribers: cfe-commits, hiraditya. Herald added a project: clang. Changed prior to commit:

[PATCH] D92808: [ObjC][ARC] Annotate calls with attributes instead of emitting retainRV or claimRV calls in the IR

2021-01-25 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments. Comment at: llvm/lib/IR/Instruction.cpp:580 +if (auto *CB = dyn_cast(this)) + return objcarc::hasRetainRVOrClaimRVAttr(CB); +return false; This change looks pretty fishy. Objective C shouldn't be hijacking LLVMs core ins

[PATCH] D94827: [SimplifyCFG] If provided, preserve Dominator Tree

2021-01-27 Thread Nikita Popov via Phabricator via cfe-commits
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/D94827/new/ https://reviews.llvm.org/D94827 ___ cfe

[PATCH] D93040: [InlineFunction] Use llvm.experimental.noalias.scope.decl for noalias arguments.

2021-02-01 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:929 -if (MDNode *M = NI->getMetadata(LLVMContext::MD_alias_scope)) - NI->setMetadata(LLVMContext::MD_alias_scope, MDMap[M]); +if (MDNode *M = I->getMetadata(LLVMContext::MD_alias_

[PATCH] D95849: [FileCheck] Default --allow-unused-prefixes to false

2021-02-02 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Just to be clear here: Only the small handful where a spelling mistake was fixed were actually bugs. All other unused check prefixes were there for convenience, and are now casualties of this unnecessary crusade. I regret not speaking out against this at the time. Repos

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

2021-02-05 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Herald added a reviewer: jansvoboda11. @guiand Do you plan to continue working on this patch? If not, I would try to finalize it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81678/new/ https://reviews.llvm.org/D81678

[PATCH] D110445: [NFC] Avoid some AttrBuilder redundant initialization

2021-09-24 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Wouldn't this go against https://llvm.org/docs/CodingStandards.html#do-not-use-static-constructors? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110445/new/ https://reviews.llvm.org/D110445

[PATCH] D110745: Redefine deref(N) attribute family as point-in-time semantics (aka deref-at-point)

2021-10-01 Thread Nikita Popov via Phabricator via cfe-commits
nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. Sorry, but the fact that there is still no way to opt-in to the old behavior is still a blocker from my side. If we can't use `dereferenceable + nofree` arguments for that purpose, the

[PATCH] D94376: [MemCpyOpt] Enable MemorySSA by default

2021-02-19 Thread Nikita Popov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. nikic marked an inline comment as done. Closed by commit rG71a8e4e7d6b9: [MemCopyOpt] Enable MemorySSA by default (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to

[PATCH] D94376: [MemCpyOpt] Enable MemorySSA by default

2021-02-22 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. @uabelho Thanks for the report! This issue should be resolved by https://github.com/llvm/llvm-project/commit/4125afc35723b490556f7a38f7835f0914f70292. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94376/new/ https://reviews.

[PATCH] D103491: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC)

2021-06-03 Thread Nikita Popov via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG983565a6fe4a: [ADT] Move DenseMapInfo for ArrayRef/StringRef into respective headers (NFC) (authored by nikic). Changed prior to commit: https://r

[PATCH] D104099: [NewPM] Remove SpeculateAroundPHIs pass from pipeline

2021-06-11 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Looks like phase ordering tests need an update. Comment at: llvm/lib/Passes/PassBuilder.cpp:1449 - // inserting redundancies into the program. This even includes SimplifyCFG. - OptimizePM.addPass(SpeculateAroundPHIsPass()); - As it has

[PATCH] D105877: [Coroutines] Run coroutine passes by default

2021-08-03 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. I noticed that this change had a measurable impact on `O0` memory usage, which I wouldn't have expected (https://llvm-compile-time-tracker.com/compare.php?from=0f9e6451a836886f39137818c4f0cfd69ae31e62&to=8a1727ba51d262365b0d9fe10fef7e50da7022cd&stat=max-rss). Any idea wha

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

2021-08-16 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D106891#2945342 , @arsenm wrote: > I don’t think constructing in the pass is the solution. Why exactly is this > introducing such a big slowdown? The reason are the additional DominatorTree and LoopInfo calculations. These hav

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

2021-08-16 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. You also need to drop the `getAnalysisUsage()` change, otherwise the pass manager will still create the ORE itself. PS: This revision still has incorrectly configured permissions, which prevents me from leaving line comments. Revisions on this Phabricator instance should

[PATCH] D108073: [PassBuilder] Don't use MemorySSA for standalone LoopRotate passes

2021-08-16 Thread Nikita Popov via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0a031449b2c7: [PassBuilder] Don't use MemorySSA for standalone LoopRotate passes (authored by nikic). Herald added a project: clang. Herald added a s

[PATCH] D108074: [MemorySSA] Remove unnecessary MSSA dependencies

2021-08-16 Thread Nikita Popov via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG570c9beb8ebb: [MemorySSA] Remove unnecessary MSSA dependencies (authored by nikic). Herald added subscribers: cfe-commits, ormris, steven_wu. Herald

[PATCH] D104790: [x86] fix mm*_undefined* intrinsics to use arbitrary frozen bit pattern

2021-06-26 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Is this actually better in any meaningful way? InstCombine will turn `freeze poison` into `zeroinitializer`, and until then this is just a completely opaque value. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104790/new/ h

[PATCH] D105395: [IRBuilder] Add type argument to CreateMaskedLoad/Gather

2021-07-03 Thread Nikita Popov via Phabricator via cfe-commits
nikic created this revision. nikic added a reviewer: opaque-pointers. Herald added subscribers: dcaballe, cota, teijeong, dexonsmith, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, antiagainst, shauheen, rriddle, mehdi_a

[PATCH] D105395: [IRBuilder] Add type argument to CreateMaskedLoad/Gather

2021-07-03 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments. Comment at: mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td:1519 string llvmBuilder = [{ +llvm::Type *Ty = $data->getType()->getPointerElementType(); $res = $pass_thru.empty() ? builder.CreateMaskedLoad( Would be great if some

[PATCH] D105395: [IRBuilder] Add type argument to CreateMaskedLoad/Gather

2021-07-04 Thread Nikita Popov via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfabc17192ec1: [IRBuilder] Add type argument to CreateMaskedLoad/Gather (authored by nikic). Changed prior to commit: https://reviews.llvm.org/D105

[PATCH] D103465: [OpaquePtr] Track pointee types in Clang

2021-07-07 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Maybe send a mail to cfe-dev to solicit some help with the opaque pointer migration on the clang side? Comment at: clang/lib/CodeGen/Address.h:29-30 public: Address(llvm::Value *pointer, CharUnits alignment) - : Pointer(pointer), Alignment(align

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

2021-11-03 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. I'd still like an answer to: In D112041#3073464 , @nikic wrote: > Is it actually necessary to thread this through AssumptionCache, given how > InferAddressSpaces is the only place that looks at these assumes? I'd prefer not to in

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-05 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. I don't think I fully understand the interaction this has with eager invalidation. I would have expected that if we eagerly invalidate, then this fine-grained invalidation wouldn't make much of a difference, because we invalidate anyway after processing a function. ===

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

2021-11-09 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added a comment. LGTM as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113304/new/ https://reviews.llvm.org/D113304 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D94741: [Utils] Check for more global information in update_test_checks

2021-03-13 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. It looks like this has broken handling of `getelementptr %T`, and generates `getelementptr [[T]]` for it now, see e.g. the first change in https://github.com/llvm/llvm-project/commit/7ee96429a0b057bcc97331a6a762fc3cd00aed61. In https://github.com/llvm/llvm-project/commit

[PATCH] D93164: [AST] Add generator for source location introspection

2021-03-14 Thread Nikita Popov via Phabricator via cfe-commits
nikic reopened this revision. nikic added a comment. This revision is now accepted and ready to land. Reverted in https://github.com/llvm/llvm-project/commit/e0f70a8a979f5b843e90a0a20442ca79e2507208 due to build failure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D93164: [AST] Add generator for source location introspection

2021-03-14 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. @steveire When running the command manually I get: /root/llvm-compile-time-tracker/llvm-project/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py --json-input-path /root/llvm-compile-time-tracker/llvm-project-build/ASTNodeAPI.json --output-file generated/NodeIntrosp

[PATCH] D110745: Redefine deref(N) attribute family as point-in-time semantics (aka deref-at-point)

2021-11-13 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a subscriber: aeubanks. nikic added a comment. In D110745#3128719 , @reames wrote: > @nikic ping on previous question. It's been a month, and this has been > LGTMed. Without response, I plan to land this. Sorry, I did do some measurements

[PATCH] D114394: Compile-time computation of string attribute hashes

2021-11-30 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=3608e18a946e77a474a468304b6c3904c55dbce0&to=0ce74a09cc8533a10fb67fdec4fb6ad8de4f1153&stat=instructions Some improvement at `O3`, not much change for optimized builds. A concern I have is that this may be

[PATCH] D114163: Use VersionTuple for parsing versions in Triple. This makes it possible to distinguish between "16" and "16.0" after parsing, which previously was not possible.

2021-11-30 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. FYI I reverted this due to test failures -- one specific assertion failure is mentioned in https://reviews.llvm.org/rG40d5eeac6cd8. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114163/new/ https://reviews.llvm.org/D114163

[PATCH] D136284: SROA should freeze undefs for loads with no prior stores

2022-10-19 Thread Nikita Popov via Phabricator via cfe-commits
nikic requested changes to this revision. nikic added a reviewer: nlopes. nikic added a comment. This revision now requires changes to proceed. The current behavior here is intentional -- in fact, LLVM will move towards returning poison for loads from uninitialized memory in the future (though p

[PATCH] D136284: SROA should freeze undefs for loads with no prior stores

2022-10-21 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. At least in C++, working with uninitialized memory is pretty much always immediate undefined behavior, see https://eel.is/c++draft/basic.indet for the relevant wording. The only exception are "copy-like" operations on unsigned character types, which comparisons do not fal

[PATCH] D136258: [PowerPC] Fix check for ieeelongdouble support

2022-10-27 Thread Nikita Popov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0cbf003c78cd: [PowerPC] Fix check for ieeelongdouble support (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINC

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

2022-10-29 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. I think adding this under a default-disabled flag is fine for evaluation purposes, but I have doubts that we will ever be able to enable this by default. There is a lot of code out there assuming that copying uninitialized data around is fine. Repository: rG LLVM Gith

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

2022-10-30 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D134410#3894995 , @nlopes wrote: > We wanted this patch to make us switch uninitialized loads to poison at will, > since they become UB. In practice, this helps us fixing bugs in SROA and etc > without perf degradation. Can yo

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

2022-11-01 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D134410#3895253 , @nlopes wrote: > In D134410#3895077 , @nikic wrote: > >> In D134410#3894995 , @nlopes wrote: >> >>> We wanted this patch to mak

[PATCH] D134300: [llvm] Handle dso_local_equivalent in FunctionComparator

2022-09-20 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. Please drop the clang test and use update_test_checks.py for LLVM tests. Comment at: llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll:1 +;; Check the cases involving dso_local_equivalent where we do expect functions to be merged. +; RUN: opt

[PATCH] D134316: [clang][docs] Fix supported element types for `__builtin_reduce_(add|mul)`

2022-09-21 Thread Nikita Popov via Phabricator via cfe-commits
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/D134316/new/ https://reviews.llvm.org/D134316 ___ cfe

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

2022-09-22 Thread Nikita Popov via Phabricator via cfe-commits
nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. I'd like to block this on `-fsanitize-memory-param-retval` (aka msan eager checks) being enabled by default. It seems pretty clear that there is a *lot* of UB due to uninitialized para

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

2022-09-23 Thread Nikita Popov via Phabricator via cfe-commits
nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. I have some concerns about this change: - It makes it fundamentally impossible to link against a static system-libcxxabi (or any other `LIBCXX_CXX_ABI` option). This is currently alre

<    1   2   3   4   5   >