[PATCH] D157518: Avoid running optimization passes in frontend test

wenlei accepted this revision. wenlei added a comment. lgtm, thanks. Comment at: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp:147-148 -// CHECK: ![[PROF_LIKELY]] = !{!"branch_weights", i32 [[UNLIKELY]], i32 [[LIKELY]]} -// CHECK: ![[PROF_UNLIKELY]] = !{!"branc

[PATCH] D158668: RFC: Add getLikelyBranchWeight helper function

wenlei added a comment. In D158668#4611988 , @MatzeB wrote: > And as another strawman / discussion-starter I put up D158680 > where I use `!{"branch_weights", i32 1, > i32 0}` to represent likely branches and the actua

[PATCH] D157518: Avoid running optimization passes in frontend test

wenlei added inline comments. Comment at: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp:147-148 -// CHECK: ![[PROF_LIKELY]] = !{!"branch_weights", i32 [[UNLIKELY]], i32 [[LIKELY]]} -// CHECK: ![[PROF_UNLIKELY]] = !{!"branch_weights", i32 [[LIKELY]], i32 [[UNLIKE

[PATCH] D152741: [WPD] implement -funknown-vtable-visibility-filepaths

wenlei added a comment. > For concrete data are you talking about between the different solutions e.g. > --lto-whole-program-visibility, -funknown-vtable-visibility-filepaths, RTTI > based, FatLTO based etc or something else? Right, between the different solutions. RTTI based solution doesn't e

[PATCH] D152741: [WPD] implement -funknown-vtable-visibility-filepaths

wenlei added a comment. > The big advantage of doing this in the FE is that we know which types are > actually coming from the native headers. Blocking all types in the TU is > overly conservative and also less stable as header changes can effectively > turn on/off unrelated large chunks of WPD

[PATCH] D83906: [CodeGen] Emit a call instruction instead of an invoke if the called llvm function is marked nounwind

wenlei added a comment. > I wonder if this is just a single example, where there could be various other > (header-related) peepholes that cause similar problems for stable output. > IIRC, the usual Clang approach is to make as-close-to-optimal IR up front, > but maybe in some situations it's de

[PATCH] D142994: [UsersManual] Add llvm-progen as an alternative tool for AutoFDO profile generation.

wenlei added a subscriber: davidxl. wenlei added a comment. Given llvm-profgen is in llvm tree, I'm wondering if it makes sense to treat it first option instead of alternative to out of tree create_llvm_prof. wdyt @davidxl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D136846: [Driver] Add -fsample-profile-use-profi

wenlei added a comment. In D136846#3910272 , @MaskRay wrote: > In D136846#3890699 , @wenlei wrote: > >> Did you see measurable perf boost with profi on autofdo? Or what motivated >> you to turn on profi? In most

[PATCH] D136846: [Driver] Add -fsample-profile-use-profi

wenlei added a comment. Did you see measurable perf boost with profi on autofdo? Or what motivated you to turn on profi? In most cases, profi helps when csspgo is used (instead of traditional autofdo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D136846: [Driver] Enable profi flag as user-facing flag

wenlei added a comment. I'm curious why do you need profi to be a driver flag? There're many similar tuning flags that don't have corresponding driver flag. profi is narrower than most tuning flags as it's specific to one component (profile inference) unique to sample PGO. Repository: rG LL

[PATCH] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

wenlei added a subscriber: davidxl. wenlei added a comment. For instr PGO, changing behavior may have perf implication. cc @davidxl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134456/new/ https://reviews.llvm.org/D134456

[PATCH] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

wenlei added a comment. There's also subtle difference between different variants of PGO. For instrumentation PGO where profile quality is higher, profile counts always take precedence. But for sampling PGO (AutoFDO, CSSPGO), it tries to not overwrite any existing branch weights, which means th

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

wenlei added a subscriber: weiwang. wenlei added a comment. +@weiwang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125291/new/ https://reviews.llvm.org/D125291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D121969: Pass split-machine-functions to code generator when flto is used

wenlei added a comment. The linter warning seems legit, otherwise looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121969/new/ https://reviews.llvm.org/D121969 ___ cfe-commits mailing list cfe-c

[PATCH] D121969: Pass split-machine-functions to code generator when flto is used

wenlei added inline comments. Comment at: clang/test/Driver/split-machine-functions.c:2 +// Split machine functions only work for ELF, so disable the test on Windows +// UNSUPPORTED: system-windows + My understanding is that if you specify target, e.g. `-target

[PATCH] D120610: [DebugInfo] Include DW_TAG_skeleton_unit when looking for parent UnitDie

This revision was automatically updated to reflect the committed changes. Closed by commit rG4f320ca4ba15: [DebugInfo] Include DW_TAG_skeleton_unit when looking for parent UnitDie (authored by wenlei). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D120610: [DebugInfo] Include DW_TAG_skeleton_unit when looking for parent UnitDie

wenlei added a comment. In D120610#3376002 , @dblaikie wrote: > It'd be good to include some testing beyond "does not crash" - like what was > the specific debug info we were trying to create when the crash was hit? > Perhaps we should be testing that (

[PATCH] D120610: [DebugInfo] Include DW_TAG_skeleton_unit when looking for parent UnitDie

wenlei updated this revision to Diff 414737. wenlei added a comment. Add test check for template type parameters Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120610/new/ https://reviews.llvm.org/D120610 Files: llvm/lib/CodeGen/AsmPrinter/DIE.cp

[PATCH] D120610: [DebugInfo] Include DW_TAG_skeleton_unit when looking for parent UnitDie

wenlei added a comment. In D120610#3373042 , @dblaikie wrote: > Fixes in LLVM require tests in LLVM - probably taking the clang test and > compiling that to llvm IR (include the original C++ source in a comment in > the IR test case) and then testing it

[PATCH] D120610: [DebugInfo] Include DW_TAG_skeleton_unit when looking for parent UnitDie

wenlei updated this revision to Diff 414606. wenlei added a comment. update test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120610/new/ https://reviews.llvm.org/D120610 Files: llvm/lib/CodeGen/AsmPrinter/DIE.cpp llvm/test/DebugInfo/X86

[PATCH] D120610: [DebugInfo] Include DW_TAG_skeleton_unit when looking for parent UnitDie

wenlei added a comment. In D120610#3348997 , @dblaikie wrote: > This should include a test - and could you describe more about where this > issue came up/what motivated the change? Sure, sorry for the delay as I just got back to this today. I sent the f

[PATCH] D120610: [DebugInfo] Include DW_TAG_skeleton_unit when looking for parent UnitDie

wenlei updated this revision to Diff 414251. wenlei added a comment. Herald added projects: clang, All. Herald added a subscriber: cfe-commits. Add test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120610/new/ https://reviews.llvm.org/D12061

[PATCH] D114130: [Clang] Add option to disable -mconstructor-aliases with -mno-constructor-aliases

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

[PATCH] D114130: [Clang] Add option to disable -mconstructor-aliases with -mno-constructor-aliases

wenlei added inline comments. Comment at: clang/include/clang/Driver/Options.td:5058-5061 + CodeGenOpts<"CXXCtorDtorAliases">, + DefaultFalse, + PosFlag, + NegFlag, nit: use two lines for consistency. check `sanitize_stats`, `sanitize_cfi_cross_dso` and othe

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

wenlei accepted this revision. wenlei added a comment. This revision is now accepted and ready to land. lgtm, thanks. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52 +namespace { +using namespace llvm::ore; mtrofin wrote: > wenlei wrote: > > curious why do

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

wenlei added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52 +namespace { +using namespace llvm::ore; curious why do we need anonymous namespace here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D110209: [CSSPGO] Set PseudoProbeInserter as a default pass.

wenlei accepted this revision. wenlei added a comment. This revision is now accepted and ready to land. thanks for the change. control this through metadata is more reliable than through LTO time flags. Scheduling a non-op pass shouldn't incur overhead either. lgtm. Repository: rG LLVM Githu

[PATCH] D109638: [CSSPGO][llvm-profgen] Truncate stack samples with invalid return address.

wenlei accepted this revision. wenlei added a comment. This revision is now accepted and ready to land. lgtm except two nits. Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:772 +void PerfReaderBase::emitAccumulatedWarnings() { + for (auto Address : InvalidReturnAddresses

[PATCH] D109531: [CSSPGO] Enable pseudo probe instrumentation in O0 mode.

wenlei accepted this revision. wenlei added a comment. lgtm, thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109531/new/ https://reviews.llvm.org/D109531 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D109638: [CSSPGO][llvm-profgen] Truncate stack samples with invalid return address.

wenlei added a comment. > It isn't common when the program is built with the frame pointer omission > disabled, but can still happen with third-party static libs built with frame > pointer omitted. Did you mean disabled -> enabled? > This could happen to frame-pointer-based unwinding and the c

[PATCH] D109531: [CSSPGO] Enable pseudo probe instrumentation in O0 mode.

wenlei added a comment. > More specifically, with the following command, both cc1 and lld will run in > default mode, which is -O0 for cc1 and -O2 for lld. > > clang -flto 1.cpp -v -fuse-ld=lld I'm wondering is this the expected behavior or an oversight of pass pipeline setup? In what scenari

[PATCH] D109531: [CSSPGO] Enable pseudo probe instrumentation in O0 mode.

wenlei added a comment. The change makes sense given instr PGO also happens for O0. But practically, if a file is being built with O0, do we care about its profile given we're not really optimizing it anyways? Functions from O0 modules are not supposed to be inlined into O1

[PATCH] D109386: Fix use-after-free from GlobalCtors associated data

wenlei added a comment. Yes, this looks like the kind of stuff VH is designed for. @wlei here's an example of how VH is used to handle deletion of BFI of basic blocks: https://reviews.llvm.org/D75341. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D109175: [openmp] Add clang cc1 option -fopenmp-skip-deferred-diags

wenlei added a comment. In D109175#2980446 , @lebedev.ri wrote: > In D109175#2980333 , @weiwang wrote: > >> Our internal codebase never uses the target directive. Once the deferred >> diags is bypassed, we obser

[PATCH] D108905: [ItaniumCXXABI] Set nounwind on __cxa_begin_catch/__cxa_end_catch

wenlei added subscribers: modimo, wenlei. wenlei added a comment. +@modimo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108905/new/ https://reviews.llvm.org/D108905 ___ cfe-commits mailing list cfe-comm

[PATCH] D107878: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO) profile loader

wenlei added inline comments. Comment at: llvm/test/CodeGen/X86/fsafdo_test2.ll:3 +; RUN: llvm-profdata merge --sample -profile-isfs -o %t.afdo %S/Inputs/fsloader.afdo +; RUN: llc -enable-fs-discriminator -fs-profile-file=%t.afdo -show-fs-branchprob -disable-ra-fsprofile-loader

[PATCH] D107876: [CSSPGO] Allow the use of debug-info-for-profiling and pseudo-probe-for-profiling together

wenlei accepted this revision. wenlei added a comment. This revision is now accepted and ready to land. lgtm, thanks. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3896 - // These two forms of profiling info can't be used together. - if (const Arg *A1 = Args.getLastArg(o

[PATCH] D107876: [CSSPGO] Allow the use of debug-info-for-profiling and pseudo-probe-for-profiling together

wenlei added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3896 - // These two forms of profiling info can't be used together. - if (const Arg *A1 = Args.getLastArg(options::OPT_fpseudo_probe_for_profiling)) -if (const Arg *A2 = Args.getLastArg(option

[PATCH] D106193: [CSSPGO] Turn on unique linkage name by default for pseudo probe.

wenlei accepted this revision. wenlei added a comment. This revision is now accepted and ready to land. lgtm, thanks. Comment at: clang/test/Driver/pseudo-probe.c:4 // RUN: %clang -### -fpseudo-probe-for-profiling -fdebug-info-for-profiling %s 2>&1 | FileCheck %s --check-pref

[PATCH] D106193: [CSSPGO] Turn on unique linkage name by default for pseudo probe.

wenlei added inline comments. Comment at: clang/test/Driver/pseudo-probe.c:4 // RUN: %clang -### -fpseudo-probe-for-profiling -fdebug-info-for-profiling %s 2>&1 | FileCheck %s --check-prefix=CONFLICT +// RUN: %clang -### -fpseudo-probe-for-profiling -funique-internal-linkage-n

[PATCH] D106193: [CSSPGO] Turn on unique linkage name by default for pseudo probe.

wenlei added inline comments. Comment at: clang/test/Driver/pseudo-probe.c:4 // RUN: %clang -### -fpseudo-probe-for-profiling -fdebug-info-for-profiling %s 2>&1 | FileCheck %s --check-prefix=CONFLICT +// RUN: %clang -### -fpseudo-probe-for-profiling -funique-internal-linkage-n

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

wenlei added a comment. In D104099#2814167 , @davidxl wrote: > Adding Wei to help measure performance impact on our internal workloads. > Also add Wenlei to help measure impact with FB's workloads. Measured perf using FB internal workload w/ and w/o th

[PATCH] D103909: [CSSPGO] Emit mangled dwarf names for line tables debug option under -fpseudo-probe-for-profiling

wenlei accepted this revision. wenlei added a comment. This revision is now accepted and ready to land. lgtm, thanks for the fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103909/new/ https://reviews.llvm.org/D103909 __

[PATCH] D99554: [ThinLTO] During module importing, close one source module before open another one for distributed mode.

wenlei added subscribers: weiwang, wenlei. wenlei added a comment. Does this help non-distributed ThinLTO as well? cc: @weiwang Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99554/new/ https://reviews.llvm.org/D99554 __

[PATCH] D95271: [CSSPGO] Passing the clang driver switch -fpseudo-probe-for-profiling to the linker.

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

[PATCH] D95271: [CSSPGO] Passing the clang driver switch -fpseudo-probe-for-profiling to the linker.

wenlei added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:609 + // Pass an option to enable pseudo probe emission. + if (Args.hasArg(options::OPT_fpseudo_probe_for_profiling)) +CmdArgs.push_back("-plugin-opt=pseudo-probe-for-profiling"); -

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

wenlei added a subscriber: lxfind. wenlei added a comment. In D93747#2475852 , @dblaikie wrote: > In D93747#2470504 , @hoy wrote: > >> In D93747#2470387 , @dblaikie wrote: >>

[PATCH] D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it

wenlei added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:412 + + Remark << ";"; } modimo wrote: > wenlei wrote: > > nit: any special reason for adding this? doesn't seem consistent with other > > remarks we have. > If you grab the remark o

[PATCH] D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it

wenlei added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:412 + + Remark << ";"; } nit: any special reason for adding this? doesn't seem consistent with other remarks we have. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D90507: Adding DWARF64 clang flag

wenlei added a comment. This change covers non-LTO cases. For LTO, I think we would need to pass it from driver to LTO. Something like this: tools::addLTOOptions -> lld -> lto::Config (Config->TargetOptions->MCTargetOptions) ->LTO Backend. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D89066: [Coroutine][Sema] Only tighten the suspend call temp lifetime for final awaiter

wenlei accepted this revision. wenlei added a comment. This revision is now accepted and ready to land. LGTM. I think we can get this in first to address the fall out from https://reviews.llvm.org/D87470, while investigating ASAN failure. Comment at: clang/lib/Sema/SemaCorouti

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

wenlei added subscribers: asbirlea, modimo. wenlei added a comment. Just saw this. Thanks @Alina Sbirlea for fixing the tests, much appreciated! On 9/15/20, 6:44 PM, "Nico Weber via Phabricator" wrote: thakis added a comment. Looks like this breaks tests: https://urldefense.proofpoint

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

wenlei added a comment. Committed on behalf of @modimo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG2ea4c2c598b7: [BFI] Make BFI information available through loop passes inside… (authored by wenlei). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/

[PATCH] D77925: Revert "[TLI] Per-function fveclib for math library used for vectorization"

wenlei added a comment. In D77925#2236519 , @tejohnson wrote: > In D77925#2229484 , @mehdi_amini > wrote: > >> Overall that would likely work for XLA. Something I'd like to mention though >> in response to: >> >>>

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

wenlei added a comment. In D86156#2243393 , @asbirlea wrote: > Diff looks reasonable at this point. Thank you for the patch! > Please wait on @nikic for compile-time impact or additional feedback. > > Just out of curiosity, in D65060

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

wenlei added a comment. > If only LICM benefits from it, I'd consider creating a BFI instance for the > duration of the LICM pass Currently only LICM uses BFI, but I think it'd be beneficial for other loop passes to be able to use BFI too. BFI not accessible to loop passes seems to be a non-tr

[PATCH] D77925: Revert "[TLI] Per-function fveclib for math library used for vectorization"

wenlei added a comment. In D77925#2220177 , @wenlei wrote: > In D77925#2220169 , @mehdi_amini > wrote: > >> I rather have a non closed list of veclib: if you just have a map keyed by >> string instead of an enum i

[PATCH] D77925: Revert "[TLI] Per-function fveclib for math library used for vectorization"

wenlei added a comment. In D77925#2220169 , @mehdi_amini wrote: > I rather have a non closed list of veclib: if you just have a map keyed by > string instead of an enum it should just work out? The veclib type is also tied to the accepted values for `-fv

[PATCH] D77925: Revert "[TLI] Per-function fveclib for math library used for vectorization"

wenlei added a comment. In D77925#2016326 , @tejohnson wrote: > In D77925#2016299 , @wenlei wrote: > >> @mehdi_amini @tejohnson When can we re-land this (with tweaks)? I'm under >> the impression that a test case d

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

wenlei added a comment. In D82213#2110941 , @fhahn wrote: > That's interesting. We are also using something similar for the matrix > lowering remarks [1]: we traverse the inlining chain bottom up and emit a > remark at each step which contains the expres

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

This revision was automatically updated to reflect the committed changes. Closed by commit rG7c8a6936bf6b: [Remarks] Add callsite locations to inline remarks (authored by wenlei). Changed prior to commit: https://reviews.llvm.org/D82213?vs=272263&id=272289#toc Repository: rG LLVM Github Mono

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

wenlei marked an inline comment as done. wenlei added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:392 +if (ProfileGuidedInline) + Remark << " by profile guided inliner"; +Remark << " with " << IC; davidxl wrote: > Perhaps reword

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

wenlei updated this revision to Diff 272263. wenlei added a comment. Update remark message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82213/new/ https://reviews.llvm.org/D82213 Files: clang/test/Frontend/optimization-remark-with-hotness-new-

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

wenlei updated this revision to Diff 272240. wenlei added a comment. Address David's comments, add test for nested inlinining. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82213/new/ https://reviews.llvm.org/D82213 Files: clang/test/Frontend/op

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

wenlei marked an inline comment as done. wenlei added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:391 +Remark << ore::NV("Caller", &Caller); +if (ProfileGuidedInline) + Remark << " by profile guided inliner"; davidxl wrote: > is

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

wenlei created this revision. wenlei added reviewers: wmi, davidxl, hoy. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. Add call site location info into inline remarks so we can differentiate inline sites. This can be useful for inliner tuning.

[PATCH] D77925: Revert "[TLI] Per-function fveclib for math library used for vectorization"

wenlei added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. @mehdi_amini @tejohnson When can we re-land this (with tweaks)? I'm under the impression that a test case demonstrating the 3rd party usage will be added very soon after this revert, then we can tweak

[PATCH] D77952: [TLII] Reduce copies of TLII for TLA

wenlei marked 2 inline comments as done. wenlei added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:689 // Set up the per-function pass manager. - FPM.add(new TargetLibraryInfoWrapperPass(*TLII)); + FPM.add(new TargetLibraryInfoWrapperPass(TargetTriple));

[PATCH] D77952: [TLII] Reduce copies of TLII for TLA

wenlei added a comment. In D77952#1976803 , @tejohnson wrote: > Also, just a nit, because TLI is sometimes used to refer to the > TargetLibraryInfo and occasionally to the TargetLibraryInfoImpl, and the > latter is frequently referred to as TLII, could y

[PATCH] D77952: [TLI] Reduce copies for TLI and TLA

wenlei updated this revision to Diff 256806. wenlei added a comment. address feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77952/new/ https://reviews.llvm.org/D77952 Files: clang/lib/CodeGen/BackendUtil.cpp llvm/include/llvm/Analysis/

[PATCH] D77952: [TLI] Reduce copies for TLI and TLA

wenlei marked an inline comment as done. wenlei added a comment. In D77952#1976336 , @tejohnson wrote: > Some parts of this are dependent on the patch that got reverted, but I have > some other questions below about the changes in BackendUtil.cpp. Thank

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

wenlei added a comment. In D77632#1975619 , @mehdi_amini wrote: > The existing TLI provides a very convenient way to define a VecLib without > LLVM knowing about it ahead of time. This feature is important for any > embedded use of LLVM as a library out-

[PATCH] D77952: [TLI] Reduce copies for TLI and TLA

wenlei created this revision. wenlei added reviewers: tejohnson, nikic, mehdi_amini, hoyFB. Herald added subscribers: cfe-commits, hiraditya. Herald added a project: clang. Minor changes to reduce the copying needed for TLI and TLA by using move whenever possible. Repository: rG LLVM Github M

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

wenlei added a comment. In D77632#1974409 , @tejohnson wrote: > In D77632#1974363 , @wenlei wrote: > > > In D77632#1974015 , @nikic wrote: > > > > > This change causes a ~0.5

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

wenlei added a comment. In D77632#1974015 , @nikic wrote: > This change causes a ~0.5% compile-time regressions: > http://llvm-compile-time-tracker.com/compare.php?from=5b18b6e9a84d985c0a907009fb71de7c1943bc88&to=60c642e74be6af86906d9f3d982728be7bd4329f&s

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG60c642e74be6: [TLI] Per-function fveclib for math library used for vectorization (authored by wenlei). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77632/ne

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

wenlei updated this revision to Diff 256465. wenlei marked an inline comment as done. wenlei added a comment. rebase, update test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77632/new/ https://reviews.llvm.org/D77632 Files: clang/lib/CodeGen/B

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

wenlei marked 4 inline comments as done. wenlei added inline comments. Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:272 +else + VectLibrary = Impl.ClVectorLibrary; + tejohnson wrote: > wenlei wrote: > > tejohnson wrote: > > > Suggest moving

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

wenlei updated this revision to Diff 256437. wenlei marked an inline comment as done. wenlei added a comment. Herald added subscribers: haicheng, eraman. address feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77632/new/ https://reviews.llvm

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

wenlei marked 4 inline comments as done. wenlei added inline comments. Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:272 +else + VectLibrary = Impl.ClVectorLibrary; + tejohnson wrote: > Suggest moving the implementation of this constructor t

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

wenlei updated this revision to Diff 256350. wenlei added a comment. address feedback, allow functions within a module to have different vectlib setting. add test case for inline compatibility check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77

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

wenlei added a comment. Thanks for taking a look and the suggestions, @tejohnson. Yeah, you're right about the potential conflict of attributes. Initially I thought even though we now allow this to be per-function, but since it comes from per-module switch, module level consistency can still be

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

wenlei updated this revision to Diff 255703. wenlei added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77632/new/ https://reviews.llvm.org/D77632 Files: clang/lib/CodeGen/CGCall.cpp clang/test/CodeGen/libcalls-veclib.c llv

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

wenlei updated this revision to Diff 255679. wenlei added a comment. update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77632/new/ https://reviews.llvm.org/D77632 Files: clang/lib/CodeGen/CGCall.cpp clang/test/CodeGen/libcalls-veclib.c llv

[PATCH] D77484: [Vector] Pass VectLib to LTO backend so TLI build correct vector function list

wenlei added a comment. In D77484#1965976 , @tejohnson wrote: > In D77484#1965629 , @wenlei wrote: > > > > Ok then it does sound like these could be handled on a per-function > > > basis, similar to how -fno-builti

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

wenlei created this revision. Herald added subscribers: cfe-commits, dexonsmith, hiraditya. Herald added a project: clang. wenlei edited the summary of this revision. wenlei added reviewers: tejohnson, hoyFB, spatel, gchatelet. Encode `-fveclib` setting as per-function attribute so it can be threa

[PATCH] D77484: [Vector] Pass VectLib to LTO backend so TLI build correct vector function list

wenlei added a comment. > Ok then it does sound like these could be handled on a per-function basis, > similar to how -fno-builtin* are handled. I.e. a function attribute to > indicate the veclib, which would then be naturally preserved during LTO even > after merging/importing across modules.

[PATCH] D77484: [Vector] Pass VectLib to LTO backend so TLI build correct vector function list

wenlei added a comment. > Linking against two vectlibs may cause name conflicts or other issues. Of all three supported match libraries, all functions from Accelerate are prefixed with `v`; all MASS library functions are suffixed with `_massv`; and all SVML functions are prefixed with `__svml_`

[PATCH] D77484: [Vector] Pass VectLib to LTO backend so TLI build correct vector function list

wenlei added a comment. In D77484#1962445 , @tejohnson wrote: > We're trying to move towards encoding all of this in the IR. And in fact, I > recently implemented a series of patches to make the TLI to be built > per-function, and along with some patches

[PATCH] D77484: [Vector] Pass VectLib to LTO backend so TLI build correct vector function list

wenlei created this revision. wenlei added reviewers: hoyFB, spatel. Herald added subscribers: cfe-commits, dang, dexonsmith, steven_wu, MaskRay, hiraditya, arichardson, inglorion, emaste. Herald added a reviewer: espindola. Herald added a project: clang. -fveclib switch not propagated to LTO bac

[PATCH] D74814: IR printing for single function with the new pass manager.

wenlei added a comment. Committed on behalf of @hoyFB per request. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74814/new/ https://reviews.llvm.org/D74814 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D74814: IR printing for single function with the new pass manager.

This revision was automatically updated to reflect the committed changes. Closed by commit rGbae33a7c5a1f: IR printing for single function with the new pass manager. (authored by hoyFB, committed by wenlei). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D74814: IR printing for single function with the new pass manager.

wenlei accepted this revision. wenlei added a comment. This revision is now accepted and ready to land. Thanks for the making the changes, it would be nice to have some consistency (the same structure) between test cases for legacy PM and new PM, e.g. `EMPTY` is only tested for legacy PM, but no

[PATCH] D74814: IR printing for single function with the new pass manager.

wenlei added a comment. > Sounds good. I was trying to figure out how to invoke the new pass manager > with OPT. Is there a command line switch to do that? If you just do `opt -passes=`, it will invoke new pass manager for `opt`. See https://reviews.llvm.org/D66560 for example - that was for

[PATCH] D74814: IR printing for single function with the new pass manager.

wenlei added a comment. Thanks for fixing this. Could you use `.ll` IR file as test case and place it alongside with `test/Other/module-pass-printer.ll` or just reuse that one? We can also use `opt` to invoke new pass manager, so it doesn't need to be a clang test. Repository: rG LLVM Githu

[PATCH] D73060: [Clang] Fix expansion of response files in -Wp after integrated-cc1 change

wenlei accepted this revision. wenlei added a comment. This revision is now accepted and ready to land. > Is that the desired behavior? This was the previous behavior but I'm not sure > it's the right behavior. Good point. It might be safer to keep that previous behavior though. Thanks for the

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

wenlei added a comment. @aganea, @hans This patch broke response file expansion for preprocessor via `-Wp` - all our internal builds failed with this patch because we invoke clang with response file passed to preprocessor, e.g. `clang++ ... -Wp,@pp.rsp @cc.rsp`. Looks like we can't just call `E

[PATCH] D71903: [Coroutines][6/6] Clang schedules new passes

wenlei added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1228 + MPM.addPass(createModuleToFunctionPassAdaptor(CoroElidePass())); + MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(CoroSplitPass())); + MPM.addPass(createModuleToFun

[PATCH] D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO

wenlei added inline comments. Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:614 // during ThinLTO and perform the rest of the optimizations afterward. if (PrepareForThinLTO) { // Ensure we perform any last passes, but do so before renaming anonymous --

  1   2   >