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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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/

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

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] D81678: Introduce noundef attribute at call sites for stricter poison analysis

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] D95849: [FileCheck] Default --allow-unused-prefixes to false

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.

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

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 =

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

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 ___

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

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

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

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] D81678: Introduce noundef attribute at call sites for stricter poison analysis

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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] D93002: [NPM] Support -fmerge-functions

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] D93002: [NPM] Support -fmerge-functions

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:

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

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

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

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 ___

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

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

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

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 ___

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

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

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

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

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

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.

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

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

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

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

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

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=54c1bcab90102481fe43b73f8547d47446ba2163=instructions Looking at the

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

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

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

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

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] 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)

nikic added a comment. This ended up having a rather large impact... Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=567462b48eba1c2d286ce97117994463f4535d2e=e00f189d392dd9bf95f6a98f05f2d341d06cd65c=instructions Code size:

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

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

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

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

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

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

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

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:

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

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/

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

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] D87101: [X86] Update SSE/AVX ABS intrinsics to emit llvm.abs.* (PR46851)

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

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

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=cff6dda604cb0548bef5e5951dd1e74536110588=instructions Repository: rG LLVM

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

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

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

nikic added a comment. Herald added a subscriber: danielkiss. New compile-time numbers: https://llvm-compile-time-tracker.com/compare.php?from=d7c119d89c5f6d0789cfd0a139c80e23912c0bb0=e0a1a6cac1b982023f8ceba8285d1ee7bc96bd32=instructions The regression is now reduced to 0.2%. I assume that's

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

nikic added a comment. Current compile-time numbers show a 1% geomean regression: https://llvm-compile-time-tracker.com/compare.php?from=d7c119d89c5f6d0789cfd0a139c80e23912c0bb0=127615d90c7b4424ec83f5a8ab4256d08f7a8362=instructions I've left a comment inline with a possible cause.

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

nikic added a comment. This change has a 0.3% compile-time regression (https://llvm-compile-time-tracker.com/compare.php?from=0c55889d809027136048a0d144209a2bc282e7fc=71f3169e1baeff262583b35ef88f8fb6df7be85e=instructions) and a 0.3% max-rss regression

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

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

[PATCH] D84713: [DominatorTree] Simplify ChildrenGetter.

nikic added a comment. The regression here seems to be because operations on an empty GraphDiff don't optimize out. Not going through GraphDiff but using the same basic logic (https://github.com/llvm/llvm-project/commit/4c6a5de8131183ff88f52cc3dda67180e31501a1 -- going through a separate

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

nikic added a comment. D85684 has landed, so we can try reapplying this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83360/new/ https://reviews.llvm.org/D83360

[PATCH] D84959: [NewPM][LVI] Abandon LVI after CVP

This revision was automatically updated to reflect the committed changes. Closed by commit rG25af353b0e74: [NewPM][LVI] Abandon LVI after CVP (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D84713: [DominatorTree] Simplify ChildrenGetter.

nikic added a comment. This change had a significant negative compile-time impact: https://llvm-compile-time-tracker.com/compare.php?from=0b161def6cacff1a63d3cf1a1efe95b550814d7a=e22de4e46d1dd1aacc3a7060d24bcbe89908ba6c=instructions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

nikic added a comment. Numbers for the newest version: https://llvm-compile-time-tracker.com/compare.php?from=183342c0a9850e60dd7a004b651c83dfb3a7d25e=eabcf534fe8760d14c95b40f07c61450c819d643=instructions This is now a geomean 0.2% regressions, and I don't see any large outliers for individual

[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

nikic added a comment. In D77341#2144974 , @asbirlea wrote: > Thank you for the testing. Could you help with with instructions on how to > run the tracker myself? > My local testing showed a negligible regression for mafft and a negligible >

[PATCH] D81728: [InstCombine] Add target-specific inst combining

nikic added inline comments. Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:540 + bool instCombineIntrinsic(InstCombiner , IntrinsicInst , +Instruction **ResultI) const; + bool simplifyDemandedUseBitsIntrinsic(InstCombiner ,

[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

nikic added a comment. Numbers for the new patch: https://llvm-compile-time-tracker.com/compare.php?from=c0308fd154f9a945608bd42630dc81dce5edfb40=e6e3534e77961cfa65d36828de5c75f36a25d009=instructions The regression is definitely smaller now, but still fairly large. E.g. > 2% on mafft.

[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

nikic accepted this revision. nikic added a comment. LG from my side. New compile-time numbers: https://llvm-compile-time-tracker.com/compare.php?from=0b39d2d75275b80994dac06b7ad05031cbd09393=fd070b79e063fff2fad3cd4a467f64dfca83eb90=instructions It's nearly neutral now.

[PATCH] D83402: ParsedAttrInfo: Change spelling to use StringRef instead of const char*

nikic added a comment. This change looks reasonable to me in terms of coding style, however I'm not seeing any compile-time changes in terms of instructions retired myself

[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

nikic added inline comments. Comment at: llvm/test/Other/opt-O2-pipeline.ll:289 +; CHECK-NEXT: Branch Probability Analysis +; CHECK-NEXT: Block Frequency Analysis ; CHECK-NEXT: FunctionPass Manager hans wrote: > nikic wrote: > > Is it

[PATCH] D83013: [LPM] Port CGProfilePass from NPM to LPM

nikic requested changes to this revision. nikic added inline comments. This revision now requires changes to proceed. Comment at: llvm/test/Other/opt-O2-pipeline.ll:289 +; CHECK-NEXT: Branch Probability Analysis +; CHECK-NEXT: Block Frequency Analysis ;

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

nikic added inline comments. Comment at: llvm/lib/Transforms/Utils/CodeExtractor.cpp:932 case Attribute::NoCfCheck: + case Attribute::NoUndef: break; Not familiar with this code, but based on the placement of other similar attributes like

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

This revision was automatically updated to reflect the committed changes. Closed by commit rGf89f7da999f3: [IR] Convert null-pointer-is-valid into an enum attribute (authored by nikic). Changed prior to commit: https://reviews.llvm.org/D78862?vs=261375=264285#toc Repository: rG LLVM Github

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

nikic added a comment. In D75936#2032090 , @craig.topper wrote: > In D75936#2032078 , @sconstab wrote: > > > In D75936#2032027 , @nikic wrote: > > > > > This change causes

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

nikic added a comment. This change causes a 0.8% compile-time regression for unoptimized builds . Based on the pipeline test diffs, I expect this

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

nikic added a comment. Herald added a subscriber: stephenneuendorffer. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78862/new/ https://reviews.llvm.org/D78862 ___ cfe-commits mailing list

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

nikic updated this revision to Diff 261375. nikic added a comment. Fix rebase mistake in test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78862/new/ https://reviews.llvm.org/D78862 Files: clang/lib/CodeGen/CGCall.cpp

[PATCH] D77621: ADT: SmallVector size/capacity use word-size integers when elements are small

nikic added a comment. @browneee Looks like LLVM already defines `LLVM_PTR_SIZE` as a more portable version of `__SIZEOF_POINTER__`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77621/new/ https://reviews.llvm.org/D77621

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

nikic updated this revision to Diff 261279. nikic marked an inline comment as done. nikic added a comment. Rebase over committed NFC changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78862/new/ https://reviews.llvm.org/D78862 Files:

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

nikic added a comment. @arsenm If the `null-pointer-is-valid` attribute is moved into the data layout, I'm wondering how Clang's `-fno-delete-null-pointer-checks` option would work or what it would be replaced with. In Rust it is possible to define a custom target, which also defines a custom

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

nikic added a comment. In D78862#2003684 , @arsenm wrote: > FWIW I think this attribute should be replaced with a data layout property, > so this would eventually be removed Is this change planned more for the near term or the long term? I'm not really

[PATCH] D78862: [IR] Convert null-pointer-is-valid into an enum attribute

nikic updated this revision to Diff 260105. nikic added a comment. Herald added subscribers: cfe-commits, Kayjukh, frgossen, grosul1, Joonsoo, liufengdb, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, jpienaar, rriddle, mehdi_amini, asbirlea, jfb, george.burgess.iv.

[PATCH] D77621: ADT: SmallVector size/capacity use word-size integers when elements are small

nikic added inline comments. Comment at: llvm/include/llvm/ADT/SmallVector.h:84 +template const size_t SmallVectorBase::SizeTypeMax; + dblaikie wrote: > nikic wrote: > > Is this needed? I don't think it makes a lot of sense to allow odr-use of > >

[PATCH] D77621: ADT: SmallVector size/capacity use word-size integers when elements are small

nikic accepted this revision. nikic added inline comments. Comment at: llvm/include/llvm/ADT/SmallVector.h:19 #include "llvm/Support/Compiler.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/MathExtras.h" Is this include still needed?

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

nikic added a comment. In D77621#2000237 , @nikic wrote: > > Perhaps you could move the value computation into a constexpr variable & > > just return that as needed. (could be a static local constexpr, I guess - > > to avoid the issues around linkage of

[PATCH] D77621: Change BitcodeWriter buffer to std::vector instead of SmallVector.

nikic added a comment. In D77621#157 , @dblaikie wrote: > @nikic any sense of the noise floor/level on these measurements? It doesn't > /look/ like there's much left in this that would cause problems. & I assume > these measurements were made on an

[PATCH] D77621: ADT: SmallVector size & capacity use word-size integers when elements are small.

nikic reopened this revision. nikic added a comment. This revision is now accepted and ready to land. I have reverted this change, because it causes a 1% compile-time

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

nikic added a comment. I gave D77952 a try (on top of this one), but didn't see a significant improvement from that change. Looking at the callgrind output for compilation of a **small** file, I see 52M total instructions, 4 calls to TLII initialization, where

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

nikic added a comment. This change causes a ~0.5% compile-time regressions: http://llvm-compile-time-tracker.com/compare.php?from=5b18b6e9a84d985c0a907009fb71de7c1943bc88=60c642e74be6af86906d9f3d982728be7bd4329f=instructions This is quite a lot as these things go, so it would be great if you

[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

nikic added a comment. This change causes a 2.5% compile-time regression across the board, see http://llvm-compile-time-tracker.com/compare.php?from=37bcf2df01cfa47e4509a5d225a23e2ca95005e6=a90374988e4eb8c50d91e11f4e61cdbd5debb235=instructions. Can you please try to find a cheaper way to

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

nikic added a comment. In D74183#1941741 , @efriedma wrote: > If it's just tramp3d-v4, I'm not that concerned... but that's a weird result. > On x86 in particular, alignment markings have almost no effect on > optimization, generally. I've just

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

nikic added a comment. For the record, this change caused an 1.5% compile-time regression on tramp3d-v4 (http://llvm-compile-time-tracker.com/compare.php?from=43a6d285bfead762ac472a6e62beedc9f88bce89=de98cf92e301ab559a7417f1eca5cfa53624c9e1=instructions). As there was also a 0.9% increase in

[PATCH] D74787: [IRBuilder] Always respect inserter/folder

This revision was automatically updated to reflect the committed changes. Closed by commit rGf12fb2d99b8d: [IRBuilder] Always respect inserter/folder (authored by nikic). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74787/new/

[PATCH] D74787: [IRBuilder] Always respect inserter/folder

nikic marked an inline comment as done. nikic added inline comments. Comment at: llvm/test/Transforms/InstCombine/saturating-add-sub.ll:2 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py -; RUN: opt < %s -instcombine -S | FileCheck %s +; RUN: opt < %s

[PATCH] D74787: [IRBuilder] Always respect inserter/folder

nikic created this revision. nikic added reviewers: nhaehnle, Meinersbur, spatel, lebedev.ri. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added a reviewer: jdoerfert. Herald added projects: clang, LLVM. Some IRBuilder methods that were originally defined on

[PATCH] D73835: [IRBuilder] Virtualize IRBuilder

This revision was automatically updated to reflect the committed changes. Closed by commit rG3eaa53e80543: Reapply [IRBuilder] Virtualize IRBuilder (authored by nikic). Changed prior to commit: https://reviews.llvm.org/D73835?vs=244871=245004#toc Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D73835: [IRBuilder] Virtualize IRBuilder

nikic added a comment. Tried what happens if the copy constructor is deleted: https://gist.github.com/nikic/1e6ab5bbf42cfe48e7b848e60a2df180 It looks like the referenced occurrence in DIBuilder was the only place that was using the copy constructor "for good reason", most of the other uses

[PATCH] D73835: [IRBuilder] Virtualize IRBuilder

nikic reopened this revision. nikic added a comment. This revision is now accepted and ready to land. Reverted because of failures on http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/22164. Not sure why this is the only builder that's failing, but I believe the

[PATCH] D73835: [IRBuilder] Virtualize IRBuilder

This revision was automatically updated to reflect the committed changes. Closed by commit rG0765d3824d06: [IRBuilder] Virtualize IRBuilder (authored by nikic). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

nikic added inline comments. Comment at: clang/trunk/unittests/AST/ASTImporterTest.cpp:4054 +} } shafik wrote: > aganea wrote: > > Fixes > > ``` > > [2097/2979] Building CXX object > >

[PATCH] D61046: Fix compilation warnings when compiling with GCC 7.3

nikic added inline comments. Comment at: llvm/trunk/unittests/IR/ConstantRangeTest.cpp:398-401 +#if defined(__GNUC__) && __GNUC__ >= 7 +// Silence warning: variable 'HaveInterrupt3' set but not used +(void) +#endif tstellar wrote: > Same here, no

<    1   2   3   4   5