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

tejohnson added a comment. In D155997#4570562 , @aeubanks wrote: > can we try not gating this on PGO as suggested? minimizing differences > between pipelines is nice, and as mentioned it'll help with other cases Sorry for the delay, was OOO for a bit an

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

tejohnson added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155997/new/ https://reviews.llvm.org/D155997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D156580: [FunctionImport] Reduce string duplication (NFC)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG65e57bbed06d: [FunctionImport] Reduce string duplication (NFC) (authored by tejohnson). Changed prior to commit: https://reviews.llvm.org/D156580?

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

tejohnson updated this revision to Diff 545318. tejohnson marked an inline comment as done. tejohnson added a comment. Address test comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155997/new/ https://reviews.llvm.org/D155997 Files: clang/

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

tejohnson marked 2 inline comments as done. tejohnson added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/simplifycfg-speculate-blocks.ll:83 +!4 = !{i32 1, !"wchar_size", i32 4} +!5 = !{i32 8, !"PIC Level", i32 2} +!6 = !{i32 7, !"PIE Level", i32 2} ---

[PATCH] D156580: [FunctionImport] Reduce string duplication (NFC)

tejohnson created this revision. tejohnson added a reviewer: mtrofin. Herald added subscribers: hoy, ormris, steven_wu, hiraditya. Herald added a project: All. tejohnson requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits. The import/export

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

tejohnson added a comment. In D155997#4543156 , @nikic wrote: > I'm not a fan of the PGO conditional behavior here. I'd prefer to disable > speculation unconditionally if that is feasible. This is basically what > D153392

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

tejohnson updated this revision to Diff 544109. tejohnson added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155997/new/ https://reviews.llvm.org/D155997 Files: clang/test/CodeGen/pgo-sample-preparation.c llvm/lib/

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

tejohnson added a comment. In D155997#4529861 , @aeubanks wrote: > the pipeline change and simplifycfg change should be split into two changes SimplifyCFG change split into D156194 . Repository: rG LLVM Github Monore

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

tejohnson added inline comments. Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:277 +static cl::opt AlwaysAllowSimplifyCFGSpeculation( +"always-allow-simplify-cfg-speculation", cl::init(false), cl::Hidden, aeubanks wrote: > can we just drop the flag a

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

tejohnson created this revision. tejohnson added reviewers: aeubanks, nikic. Herald added subscribers: khei4, wlei, StephenFan, wenlei, hiraditya. Herald added a project: All. tejohnson requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits. S

[PATCH] D154856: [MemProf] Use new option/pass for profile feedback and matching

tejohnson added a comment. In D154856#4494195 , @Hahnfeld wrote: > Hi @tejohnson, I'm seeing crashes after this revision: > > Failed Tests (2): > LLVM-Unit :: Passes/./PluginsTests/0/2 > LLVM-Unit :: Passes/./PluginsTests/1/2 I haven't seen the

[PATCH] D154856: [MemProf] Use new option/pass for profile feedback and matching

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb4a82b62258c: [MemProf] Use new option/pass for profile feedback and matching (authored by tejohnson). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D154856: [MemProf] Use new option/pass for profile feedback and matching

tejohnson updated this revision to Diff 538805. tejohnson added a comment. Rebase on D154872 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154856/new/ https://reviews.llvm.org/D154856 Files: clang/include/clang

[PATCH] D154856: [MemProf] Use new option/pass for profile feedback and matching

tejohnson added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/MemProfiler.cpp:912 +const TargetLibraryInfo &TLI = FAM.getResult(F); +readMemprof(M, F, MemProfReader.get(), TLI); + } tejohnson wrote: > snehasish wrote: > > I think we ca

[PATCH] D154856: [MemProf] Use new option/pass for profile feedback and matching

tejohnson added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/MemProfiler.cpp:912 +const TargetLibraryInfo &TLI = FAM.getResult(F); +readMemprof(M, F, MemProfReader.get(), TLI); + } snehasish wrote: > I think we can we split this patch

[PATCH] D154856: [MemProf] Use new option/pass for profile feedback and matching

tejohnson created this revision. tejohnson added reviewers: snehasish, davidxl. Herald added subscribers: wlei, Enna1, ormris, wenlei, steven_wu, hiraditya. Herald added a project: All. tejohnson requested review of this revision. Herald added a subscriber: MaskRay. Herald added projects: clang, LL

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

tejohnson accepted this revision. tejohnson 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/D152741/new/ https://reviews.llvm.org/D152741 _

[PATCH] D153468: [clang][LTO] Add flag to run verifier after every pass

tejohnson accepted this revision. tejohnson 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/D153468/new/ https://reviews.llvm.org/D153468 _

[PATCH] D153468: [clang][LTO] Add flag to run verifier after every pass

tejohnson added inline comments. Comment at: clang/test/CodeGen/verify-each.c:5 +// RUN: %clang_cc1 -O2 -o /dev/null -triple x86_64-unknown-linux-gnu -emit-llvm-bc %s -fdebug-pass-manager 2>&1 | FileCheck %s --check-prefix=NO +// NO-NOT: Verifying + Huh, really

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

tejohnson added a comment. In D152741#4422462 , @modimo wrote: > In D152741#4421067 , @tejohnson > wrote: > >> In D152741#4419366 , @modimo wrote: >> >>> In D152741#44193

[PATCH] D146777: [clang] Preliminary fat-lto-object support

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/test/Driver/fatlto.c:1 + Is this empty file meant to contain something? Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D146777: [clang] Preliminary fat-lto-object support

tejohnson added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1065 + if (CodeGenOpts.FatLTO) { +// Set EnableSplitLTOUnit, since the config above won't +if (!TheModule->getModuleFlag("EnableSplitLTOUnit")) Can you expand the comment a bi

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

tejohnson added a comment. In D152741#4419366 , @modimo wrote: > In D152741#4419324 , @tejohnson > wrote: > >> In D152741#4419265 , @modimo wrote: >> >>> In D152741#44188

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

tejohnson added a comment. In D152741#4419265 , @modimo wrote: > In D152741#4418831 , @tejohnson > wrote: > >> I think I understand the motivation, but not sure I agree this is the right >> approach - can you si

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

tejohnson added a comment. I think I understand the motivation, but not sure I agree this is the right approach - can you simply not pass -flto-unit and -fwhole-program-vtables for these files? Also, isn't this hiding possibly necessary info from WPD that might be needed for correct class hier

[PATCH] D151593: [MemProf] Clean up MemProf instrumentation pass invocation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf354e971b09c: [MemProf] Clean up MemProf instrumentation pass invocation (authored by tejohnson). Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D151593: [MemProf] Clean up MemProf instrumentation pass invocation

tejohnson added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:995 +// TODO: Consider passing the MemoryProfileOutput to the pass builder via +// the PGOOptions, and set this up there. vitalybuka wrote: > tejohnson wrote: > > vitalybuka

[PATCH] D151593: [MemProf] Clean up MemProf instrumentation pass invocation

tejohnson added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:995 +// TODO: Consider passing the MemoryProfileOutput to the pass builder via +// the PGOOptions, and set this up there. vitalybuka wrote: > if this is registerOptimizerLast

[PATCH] D151593: [MemProf] Clean up MemProf instrumentation pass invocation

tejohnson created this revision. tejohnson added a reviewer: snehasish. Herald added subscribers: ormris, hiraditya. Herald added a project: All. tejohnson requested review of this revision. Herald added projects: clang, LLVM. First, removes the invocation of the memprof instrumentation passes fro

[PATCH] D150326: [WPD] Update llvm.public.type.test after importing functions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa40b0c3e77a2: [WPD] Update llvm.public.type.test after importing functions (authored by tejohnson). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D150326: [WPD] Update llvm.public.type.test after importing functions

tejohnson updated this revision to Diff 521340. tejohnson marked an inline comment as done. tejohnson added a comment. Address comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150326/new/ https://reviews.llvm.org/D150326 Files: clang/test/C

[PATCH] D150326: [WPD] Update llvm.public.type.test after importing functions

tejohnson marked an inline comment as done. tejohnson added inline comments. Comment at: llvm/test/ThinLTO/X86/public-type-test.ll:31 ; HIDDEN-NOT: call {{.*}}@llvm.public.type.test ; HIDDEN: call {{.*}}@llvm.type.test aeubanks wrote: > should this be checked

[PATCH] D150326: [WPD] Update llvm.public.type.test after importing functions

tejohnson created this revision. tejohnson added a reviewer: aeubanks. Herald added subscribers: ormris, steven_wu, hiraditya, emaste. Herald added a project: All. tejohnson requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added projects: clang, LLVM. I not

[PATCH] D150295: [MemProf] Update hot/cold information after importing

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9e280c47588b: [MemProf] Update hot/cold information after importing (authored by tejohnson). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D150295: [MemProf] Update hot/cold information after importing

tejohnson created this revision. tejohnson added a reviewer: davidxl. Herald added subscribers: ormris, steven_wu, hiraditya. Herald added a project: All. tejohnson requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits. The support added by D

[PATCH] D149215: [MemProf] Control availability of hot/cold operator new from LTO link

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG176889868024: [MemProf] Control availability of hot/cold operator new from LTO link (authored by tejohnson). Changed prior to commit: https://revi

[PATCH] D149800: [WIP][PGO] Add ability to mark cold functions as optsize/minsize/optnone

tejohnson added a subscriber: yamauchi. tejohnson added a comment. Previously @yamauchi did a bunch of work related to this called PGSO (Profile Guided Size Optimization). See the functions in https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/Utils/SizeOpts.cpp, which are used to g

[PATCH] D149215: [MemProf] Control availability of hot/cold operator new from LTO link

tejohnson updated this revision to Diff 518721. tejohnson added a comment. Address comment (add TODO about automatically detecting support) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149215/new/ https://reviews.llvm.org/D149215 Files: clang/t

[PATCH] D149600: [MemProf] Recognize hot/cold operator new as replaceable allocations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2cc0c0de8021: [MemProf] Recognize hot/cold operator new as replaceable allocations (authored by tejohnson). Repository: rG LLVM Github Monorepo C

[PATCH] D149600: [MemProf] Recognize hot/cold operator new as replaceable allocations

tejohnson updated this revision to Diff 518526. tejohnson added a comment. Address comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149600/new/ https://reviews.llvm.org/D149600 Files: clang/lib/AST/Decl.cpp clang/test/CodeGenCXX/new_hot_c

[PATCH] D149600: [MemProf] Recognize hot/cold operator new as replaceable allocations

tejohnson added inline comments. Comment at: clang/test/CodeGenCXX/new_hot_cold.cpp:16 + +enum class __hot_cold_t : uint8_t; +namespace malloc_namespace { snehasish wrote: > Can we skip the typedef and just say `enum class __hot_cold_t : unsigned > char;`? > >

[PATCH] D149600: [MemProf] Recognize hot/cold operator new as replaceable allocations

tejohnson created this revision. tejohnson added reviewers: rsmith, snehasish. Herald added a project: All. tejohnson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Follow up to LLVM-side change to allow conversion to hot/cold hinted opera

[PATCH] D149215: [MemProf] Control availability of hot/cold operator new from LTO link

tejohnson added a comment. In D149215#4303149 , @pcc wrote: >> Adds an LTO option > > Usual question: does it need to be an option? Could the allocator expose a > symbol such as `__malloc_hot_cold` that the linker could check for in the > symbol table?

[PATCH] D149215: [MemProf] Control availability of hot/cold operator new from LTO link

tejohnson updated this revision to Diff 517678. tejohnson added a comment. Expand command and patch description for mechanism used by distributed ThinLTO Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149215/new/ https://reviews.llvm.org/D149215 Fi

[PATCH] D149215: [MemProf] Control availability of hot/cold operator new from LTO link

tejohnson updated this revision to Diff 517044. tejohnson added a comment. Remove extraneous call Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149215/new/ https://reviews.llvm.org/D149215 Files: clang/test/CodeGen/thinlto-distributed-supports-h

[PATCH] D149215: [MemProf] Control availability of hot/cold operator new from LTO link

tejohnson updated this revision to Diff 517043. tejohnson added a comment. Rebase onto latest D149117 to remove extraneous test diffs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149215/new/ https://reviews.llvm

[PATCH] D149215: [MemProf] Control availability of hot/cold operator new from LTO link

tejohnson added a comment. This was refactored out of patch 3 (the applyImport stuff that I refactored into patch 4 in D149117 ). Previously I was controlling this behavior via the MemProfContextDisambiguation pass itself, but in refactoring I realized that it

[PATCH] D149215: [MemProf] Control availability of hot/cold operator new from LTO link

tejohnson created this revision. tejohnson added a reviewer: snehasish. Herald added subscribers: hoy, ormris, arphaman, steven_wu, hiraditya, inglorion. Herald added a project: All. tejohnson requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-com

[PATCH] D72538: [ThinLTO] Add additional ThinLTO pipeline testing with new PM

tejohnson added a comment. In D72538#4296277 , @nikic wrote: > In D72538#4296119 , @tejohnson wrote: > >> In D72538#4291552 , @nikic wrote: >> >>> Would it be possible to cut

[PATCH] D72538: [ThinLTO] Add additional ThinLTO pipeline testing with new PM

tejohnson added a comment. In D72538#4291552 , @nikic wrote: > Would it be possible to cut down the Clang side tests to only check parts > that Clang controls in some way, e.g. that SLPVectorizer is enabled? This > test is something of a PITA because it

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

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:957 + // pipeline already cleaned up the frontend output. + if (Phase != ThinOrFullLTOPhase::ThinLTOPostLink

[PATCH] D145190: [memprof] Record BuildIDs in the raw profile.

tejohnson accepted this revision. tejohnson added a comment. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145190/new/ https://reviews.llvm.org/D145190 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D145951: [docs] Add more complete documentation for -f[no]split-lto-unit

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfb3b392264a0: [docs] Add more complete documentation for -f[no]split-lto-unit (authored by tejohnson). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D53891: [LTO] Add option to enable LTOUnit splitting, and disable unless needed

tejohnson added a comment. In D53891#4186894 , @tejohnson wrote: > In D53891#4186892 , @nikic wrote: > >> In D53891#2116541 , @hans wrote: >> >>> Would be possible to add som

[PATCH] D145951: [docs] Add more complete documentation for -f[no]split-lto-unit

tejohnson created this revision. tejohnson added reviewers: nikic, hans. Herald added subscribers: StephenFan, inglorion. Herald added a project: All. tejohnson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Option was added in D53891

[PATCH] D53891: [LTO] Add option to enable LTOUnit splitting, and disable unless needed

tejohnson added a comment. In D53891#4186892 , @nikic wrote: > In D53891#2116541 , @hans wrote: > >> Would be possible to add some documentation for this flag? From a quick >> search it's not clear to me what it do

[PATCH] D145190: [memprof] Record BuildIDs in the raw profile.

tejohnson accepted this revision. tejohnson added a comment. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145190/new/ https://reviews.llvm.org/D145190 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D145644: [memprof] Add scripts to automate testdata regeneration.

tejohnson accepted this revision. tejohnson 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/D145644/new/ https://reviews.llvm.org/D145644 _

[PATCH] D145644: [memprof] Add scripts to automate testdata regeneration.

tejohnson added a comment. Generally lgtm, but why did the raw profiles change size from what is currently committed? Comment at: llvm/test/Transforms/PGOProfile/Inputs/update_memprof_inputs.sh:63 + delete[] f; + + // Loop ensures the two calls to recurse have stack contexts

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

tejohnson added a comment. Sorry for the slow response. Looks pretty good, just a few minor suggestions and questions. Comment at: llvm/include/llvm/IR/MDBuilder.h:61 /// Return metadata containing two branch weights. + MDNode *createBranchWeights(uint32_t TrueWeight, ui

[PATCH] D144482: [clang][docs] Document ThinLTO options for ld64.lld

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm with one additional stale comment fix needed. Comment at: clang/docs/ThinLTO.rst:157 pruning. Cache pruning is supported with gold, ld64 and ELF and COFF lld, but

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

tejohnson added a comment. In D131306#4052878 , @paulkirth wrote: > In D131306#4052782 , @tejohnson > wrote: > >> In D131306#4037037 , @paulkirth >> wrote: >> >>> @tejoh

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

tejohnson added a comment. In D131306#4037037 , @paulkirth wrote: > @tejohnson @xur I kind of dropped the ball on these patches, but what are > your thoughts on this approach over the old(more invasive) change to the > profdata format I had prototyped b

[PATCH] D141558: [MemProf] Collect access density statistics during profiling

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGee73d240ab1d: [MemProf] Collect access density statistics during profiling (authored by tejohnson). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D141558: [MemProf] Collect access density statistics during profiling

tejohnson updated this revision to Diff 488766. tejohnson added a comment. Address comments (version change necessitated updating the tests) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141558/new/ https://reviews.llvm.org/D141558 Files: clang/

[PATCH] D141558: [MemProf] Collect access density statistics during profiling

tejohnson created this revision. tejohnson added reviewers: snehasish, davidxl. Herald added subscribers: Enna1, wenlei. Herald added a project: All. tejohnson requested review of this revision. Herald added projects: clang, Sanitizers, LLVM. Herald added subscribers: Sanitizers, cfe-commits. Trac

[PATCH] D134687: [clang] Ensure correct metadata for relative vtables

tejohnson accepted this revision. tejohnson 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/D134687/new/ https://reviews.llvm.org/D134687 _

[PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

tejohnson accepted this revision. tejohnson 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/D137217/new/ https://reviews.llvm.org/D137217 _

[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

tejohnson added inline comments. Comment at: utils/bazel/llvm-project-overlay/llvm/BUILD.bazel:918 ":Core", +":IRPrinter", ":IRReader", Per the follow on fix to D137768, I guess this one was unnecessary. Can you check if there are any o

[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

tejohnson accepted this revision. tejohnson added a comment. lgtm with one comment update still needed Comment at: llvm/lib/IR/IRPrintingPasses.cpp:9 // // PrintModulePass and PrintFunctionPass implementations. // tejohnson wrote: > Update comment Please upd

[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

tejohnson added a comment. In D138081#3934386 , @MaskRay wrote: >> This diff splits out (from LLVMCore) IR printing passes into IRPrinter. > > I haven't spent too much time reading this patch. Is this to fix layering > check for `-module-summary/-flto=th

[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

tejohnson added a comment. In D138081#3931419 , @alexander-shaposhnikov wrote: > @tejohnson - the legacy pass manager depends on the interface defined in > IRPrintingPasses.h > (https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/LegacyPassManag

[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

tejohnson added inline comments. Comment at: llvm/include/llvm/IRPrinter/IRPrintingPasses.h:33 +/// +/// Note: This pass is for use with the new pass manager. Use the create...Pass +/// functions above to create passes for use with the legacy pass manager. aeuban

[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

tejohnson added a comment. Why not just move IRPrintingPasses.{h,cpp} as is to the new directory, vs splitting up the legacy vs new pm handling into separate places? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138081/new/ https://reviews.llvm.or

[PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

tejohnson added inline comments. Comment at: lld/COFF/LTO.cpp:238 + sys::path::append(path, directory, +outputFileBaseName + ".lto." + baseName); + sys::path::remove_dots(path, true); MaskRay wrote: > What if two input bitcode fi

[PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

tejohnson added a comment. @MaskRay wondering if this is a good change to make for ELF as well, wdyt? Comment at: clang/lib/CodeGen/BackendUtil.cpp:1104 - auto AddStream = [&](size_t Task) { + auto AddStream = [&](size_t Task, Twine File) { return std::make_unique(std:

[PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

tejohnson added a comment. The naming convention used here for COFF is actually very similar to what is done for other ThinLTO save-temps output files, which are placed at the input module locations. We may want to just do this across the board. @MaskRay wdyt? A few other questions/comments bel

[PATCH] D137768: [opt][clang] Enable using -module-summary /-flto=thin with -S / -emit-llvm

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm, thanks! One small suggestion below. Comment at: clang/lib/CodeGen/BackendUtil.cpp:988 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {

[PATCH] D137768: [opt][clang] Enable using -module-summary with -S / -emit-llvm

tejohnson added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1016 case Backend_EmitLL: -MPM.addPass(PrintModulePass(*OS, "", CodeGenOpts.EmitLLVMUseLists)); +if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) { + if (!TheModule-

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

tejohnson marked an inline comment as done. tejohnson added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1247 +uint32_t Column) { + return hash_combine(Function, LineOffset, Column); +} M

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa212d8da94d0: [MemProf] Memprof profile matching and annotation (authored by tejohnson). Changed prior to commit: https://reviews.llvm.org/D128142

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

tejohnson updated this revision to Diff 456710. tejohnson marked 5 inline comments as done. tejohnson added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128142/new/ https://reviews.llvm.org/D128142 Files: clang/lib/F

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

tejohnson marked 12 inline comments as done. tejohnson added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1252 +AllocInfo->Info.getMinLifetime()); + SmallVector StackIds; + std::set StackHashSet; ---

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

tejohnson added a comment. In D131306#3756153 , @paulkirth wrote: > In D131306#3756087 , @tejohnson > wrote: > >> Well I was thinking the extra field would be optional as well and could be >> removed. But unders

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

tejohnson added a comment. In D131306#3756074 , @paulkirth wrote: > In D131306#3756009 , @tejohnson > wrote: > >> @davidxl @xur for review and thoughts. >> >> I'm a little wary of requiring that both pieces of me

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

tejohnson added subscribers: xur, davidxl. tejohnson added a comment. @davidxl @xur for review and thoughts. I'm a little wary of requiring that both pieces of metadata be carried together, as that seems very brittle to maintain in the compiler. What would happen if the MD_expected didn't get p

[PATCH] D132186: Clang: Add a new flag Wmisnoinline for printing hot noinline functions

tejohnson added a comment. In D132186#3750925 , @iamarchit123 wrote: > @paulkirth this change was done under the intuition that marking hot > functions noinline may hurt performance. This change till now hasn't been > tested for performance improvemen

[PATCH] D130531: [IR] Use Min behavior for module flag "PIC Level"

tejohnson accepted this revision. tejohnson 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/D130531/new/ https://reviews.llvm.org/D130531 _

[PATCH] D130531: [IR] Use Min behavior for module flag "PIC Level"

tejohnson added a comment. Looked into this a bit and the change does also seem correct to me. One comment: I see a test checking the upgrade behavior from the old Error type, but not for the upgrade from Max to Min. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D130531: [IR] Use Min behavior for module flag "PIC Level"

tejohnson added a comment. @tmsriram can you take a look? I can review from a code standpoint but you are more familiar with PIC/PIE details. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130531/new/ https://reviews.llvm.org/D130531 _

[PATCH] D128955: [WPD] Use new llvm.public.type.test intrinsic for potentially publicly visible classes

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm In D128955#3676478 , @aeubanks wrote: > In D128955#3676198 , @tejohnson > wrote: > >> In D128955

[PATCH] D128955: [WPD] Use new llvm.public.type.test intrinsic for potentially publicly visible classes

tejohnson added a subscriber: fhahn. tejohnson added a comment. In D128955#3674787 , @aeubanks wrote: > random question, if the old API is "legacy", are there any plans to remove it? @fhahn started to work on this at some point (see https://bugs.llvm.or

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

tejohnson added inline comments. Comment at: clang/test/CodeGen/memprof.cpp:15 +// # Collect memory profile: +// $ clang++ -fuse-ld=lld -Wl,-no-pie -Wl,--no-rosegment -gmlt \ +// -fdebug-info-for-profiling -mno-omit-leaf-frame-pointer \ snehasish wrote: > Ju

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

tejohnson updated this revision to Diff 446986. tejohnson marked 3 inline comments as done. tejohnson added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128142/new/ https://reviews.llvm.org/D128142 Files: clang/lib/F

[PATCH] D128955: [WPD] Use new llvm.public.type.test intrinsic for potentially publicly visible classes

tejohnson added inline comments. Comment at: llvm/lib/LTO/ThinLTOCodeGenerator.cpp:456 + updatePublicTypeTestCalls(TheModule, +/* WholeProgramVisibilityEnabledInLTO */ false); Add a comment to see note at call to updateVCallVisibili

[PATCH] D128955: [WPD] Use new llvm.public.type.test intrinsic for potentially publicly visible classes

tejohnson added a comment. In D128955#3666830 , @aeubanks wrote: > the assert that there are no public.type.tests in LTT fails on > `CodeGenCXX/thinlto-distributed-type-metadata.cpp`. for some reason clang > doesn't go through the LTO API at [1], it jus

[PATCH] D128955: [WPD] Use new llvm.public.type.test intrinsic for potentially publicly visible classes

tejohnson added inline comments. Comment at: clang/test/CodeGenCXX/thinlto-distributed-type-metadata.cpp:22 // OPT-NOT: @llvm.type.test -// OPT-NOT: call void @llvm.assume // We should have only one @llvm.assume call, the one that was expanded aeubanks wrote: >

[PATCH] D128955: [WPD] Use new llvm.public.type.test intrinsic for potentially publicly visible classes

tejohnson added a comment. Thanks for implementing this! In D71913 , I also modified this code, which might need something similar: https://github.com/llvm/llvm-project/blob/59afc4038b1096bc6fea7b322091f6e5e2dc0b38/clang/lib/CodeGen/ItaniumCXXABI.cpp#L707-L713

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

tejohnson updated this revision to Diff 441529. tejohnson marked 3 inline comments as done. tejohnson added a comment. Rebase on top of D128854 which now includes the extracted Analysis utilities. I have not yet addressed the other comments on this patch. Repo

  1   2   3   4   5   6   7   >