[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

tejohnson added a comment. Thanks for your patience, finally had a chance to go through everything much more carefully. Looks good, mostly a bunch of small or nitpicky final suggestions. The main comment/question of significance relates to where hasUnknownCall is being set currently. Patch tit

[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

tejohnson added inline comments. Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:575 unsigned AlwaysInline : 1; +unsigned NoUnwind : 1; +// Indicate if function contains instructions that mayThrow tejohnson wrote: > No Unwind needs a comment. A

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

tejohnson created this revision. tejohnson added reviewers: thakis, scott.linder. tejohnson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In looking at the disk space used by a ninja check-all, I found that a few of the largest files were

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

tejohnson added a comment. In D110276#3016295 , @thakis wrote: > Oh, the win failure > (https://buildkite.com/llvm-project/premerge-checks/builds/57480#f7da3275-775a-4cf4-9624-d3539dd1f709) > might actually be real. Windows doesn't allow deleting files

[PATCH] D36850: [ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation

tejohnson added a comment. Few follow ups below. Comment at: clang/test/CodeGen/thinlto-funcattr-prop.ll:16 + +; CHECK: ^2 = gv: (guid: 13959900437860518209, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, ds

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

tejohnson added a comment. In D110276#3016312 , @tejohnson wrote: > In D110276#3016295 , @thakis wrote: > >> Oh, the win failure >> (https://buildkite.com/llvm-project/premerge-checks/builds/57480#f7da3275-775a-4

[PATCH] D36850: [ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm (one minor comment issue noted below). Thanks! Comment at: clang/test/CodeGen/thinlto-funcattr-prop.ll:17 -; CHECK: ^2 = gv: (guid: 13959900437860518209, summari

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

tejohnson added a comment. In D110276#3027955 , @scott.linder wrote: > Is it possible to use soft links instead of copies? It would still be good to > clean up afterwards, but if the host won't allow that cleanup in some cases > at least we aren't leav

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd87bdc272ba4: Clean up large copies of binaries copied into temp directories in tests (authored by tejohnson). Repository: rG LLVM Github Monorepo

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

tejohnson added a comment. In D110276#3029952 , @thakis wrote: > Softlinks don't work on Windows, that's why these tests use copies. > > clang_f_opts.c sadly started flaking almost immediately after this went in: > http://45.33.8.238/win/46115/step_7.txt

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

tejohnson added a comment. In D110276#3030390 , @tejohnson wrote: > In D110276#3029952 , @thakis wrote: > >> Softlinks don't work on Windows, that's why these tests use copies. >> >> clang_f_opts.c sadly started f

[PATCH] D110750: [clang] Fix sentence in the usage section of ThinLTO docs.

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

[PATCH] D110276: Clean up large copies of binaries copied into temp directories in tests

tejohnson added a comment. In D110276#3031133 , @thakis wrote: > It's my personal bot. It doesn't send email. IMHO the problem of how to make > bots send emails hasn't been solved (they either send too much or too > little), so I'm not pretending that i

[PATCH] D96456: [ThinLTO, NewPM] Add Config::OptPassBuilderHook

tejohnson added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1549 + Conf.OptPassBuilderHook = [&](PassBuilder &PB) { +addSanitizers(TargetTriple, CGOpts, LOpts, PB); + }; I can't find where this is defined. Comment at: l

[PATCH] D96456: [ThinLTO, NewPM] Add Config::OptPassBuilderHook

tejohnson added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1066 PassBuilder::OptimizationLevel Level) { +if (CodeGenOpts.OptimizationLevel == 0) { + if (!CodeGenOpts.ThinLTOIndexFile.empty())

[PATCH] D96456: [ThinLTO, NewPM] Register sanitizers with OptimizerLastPassBuilderHook

tejohnson added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1069 + if (!CodeGenOpts.ThinLTOIndexFile.empty()) { +// ThinLTOIndexFile is provideds so we must be in ThinLTO PostLink. +// For -O0 ThinLTO PreLink does basic optimization and tri

[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

tejohnson created this revision. tejohnson added a reviewer: pcc. Herald added a subscriber: Prazek. tejohnson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When WPD is enabled, via WholeProgramVTables, emit type metadata for available_ex

[PATCH] D96456: [ThinLTO, NewPM] Register sanitizers with OptimizerLastPassBuilderHook

tejohnson added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1070-1071 +// ThinLTOIndexFile is provideds so we must be in ThinLTO PostLink. +// For -O0 ThinLTO PreLink does basic optimization and triggers +// OptimizerLastEPCallbacks. Pos

[PATCH] D96320: [ThinLTO, NewPM] Run OptimizerLastEPCallbacks from buildThinLTOPreLinkDefaultPipeline

tejohnson added a comment. I tend to like this approach better for the reasons listed in D96456 . @aeubanks any objections? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96320/new/ https://reviews.llvm.org/D96320

[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

tejohnson added a comment. In D96919#2575037 , @davidxl wrote: > So in this case, the virtual call in user code is resolved to a strong > definition in the shared library even though user code also has a derived > class defined? In other words, the libra

[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

tejohnson updated this revision to Diff 325041. tejohnson added a comment. Add comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96919/new/ https://reviews.llvm.org/D96919 Files: clang/lib/CodeGen/ItaniumCXXABI.cpp clang/test/CodeGenCXX/t

[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

tejohnson added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:1781 +// analysis. +if (VTable->isDeclarationForLinker()) + CGM.addCompilerUsedGlobal(VTable); davidxl wrote: > Is it better to guard it with with if > (CGM.getCodeGenO

[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

tejohnson updated this revision to Diff 325065. tejohnson added a comment. Add assert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96919/new/ https://reviews.llvm.org/D96919 Files: clang/lib/CodeGen/ItaniumCXXABI.cpp clang/test/CodeGenCXX/typ

[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0923a60ea70f: [clang] Emit type metadata on available_externally vtables for WPD (authored by tejohnson). Repository: rG LLVM Github Monorepo CHA

[PATCH] D96320: [ThinLTO, NewPM] Run OptimizerLastEPCallbacks from buildThinLTOPreLinkDefaultPipeline

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

[PATCH] D103579: [LTO] Fix -fwhole-program-vtables handling after HIP ThinLTO patch

tejohnson added inline comments. Comment at: clang/test/Driver/hip-options.hip:63 // RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \ -// RUN: --cuda-gpu-arch=gfx906 -foffload-lto=thin %s 2>&1 \ -// RUN: | FileCheck -check-prefix=THINLTO %s +// RUN:

[PATCH] D103579: [LTO] Fix -fwhole-program-vtables handling after HIP ThinLTO patch

tejohnson updated this revision to Diff 349647. tejohnson added a comment. Add Cuda test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103579/new/ https://reviews.llvm.org/D103579 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/

[PATCH] D103579: [LTO] Fix -fwhole-program-vtables handling after HIP ThinLTO patch

tejohnson marked an inline comment as done. tejohnson added inline comments. Comment at: clang/test/Driver/hip-options.hip:63 // RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \ -// RUN: --cuda-gpu-arch=gfx906 -foffload-lto=thin %s 2>&1 \ -// RUN: | Fi

[PATCH] D103579: [LTO] Fix -fwhole-program-vtables handling after HIP ThinLTO patch

tejohnson updated this revision to Diff 349690. tejohnson marked an inline comment as done. tejohnson added a comment. Combine check prefixes as suggested Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103579/new/ https://reviews.llvm.org/D103579 F

[PATCH] D103579: [LTO] Fix -fwhole-program-vtables handling after HIP ThinLTO patch

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd0ee8b64ecf3: [LTO] Fix -fwhole-program-vtables handling after HIP ThinLTO patch (authored by tejohnson). Repository: rG LLVM Github Monorepo CHA

[PATCH] D103048: [IR] make -stack-alignment= into a module attr

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm Comment at: llvm/test/Linker/stack-alignment.ll:11 +;--- main.ll +; NONE: error: linking module flags 'override-stack-alignment': IDs have conflicting values +; C

[PATCH] D97755: [IRSymTab] Set FB_used on llvm.compiler.used symbols

tejohnson added inline comments. Comment at: clang/test/CodeGen/thinlto-inline-asm2.c:10 + +//--- a.c +const char *ref() { Is this other file needed for the test? Comment at: llvm/test/ThinLTO/X86/asm.ll:11 +; NM-NOT: {{.}} +; NM: -

[PATCH] D97755: [IRSymTab] Set FB_used on llvm.compiler.used symbols

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm with change suggested below. Thanks! Comment at: clang/test/CodeGen/thinlto-inline-asm2.c:10 + +//--- a.c +const char *ref() { MaskRay wrote: > te

[PATCH] D114229: [clang][driver] Always add LTO options when using GNU toolchain

tejohnson added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:558 assert(!Inputs.empty() && "Must have at least one input."); addLTOOptions(ToolChain, Args, CmdArgs, Output, Inputs[0], D.getLTOMode() == LTOK_Thin);

[PATCH] D114764: Use `thin` that starts with lowercase letter. - `Thin` gives an error when following lib/Transforms/IPO/WholeProgramDevirt.cpp, and `thin` works.

tejohnson added a comment. There are some other docs that also use "Thin" (and "Full", etc). I see my own script uses "THIN" for this cmake variable. Looks like we convert to uppercase so I wouldn't think it should matter: See llvm/cmake/modules/HandleLLVMOptions.cmake:30 Can you clarify what p

[PATCH] D114764: Use `thin` that starts with lowercase letter. - `Thin` gives an error when following https://clang.llvm.org/docs/ThinLTO.html#clang-bootstrap, and `thin` works.

tejohnson added a comment. In D114764#3160212 , @luna wrote: > In D114764#3160204 , @tejohnson > wrote: > >> There are some other docs that also use "Thin" (and "Full", etc). I see my >> own script uses "THIN" f

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

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] 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] 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] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

tejohnson added a comment. I prefer the way proposed here, which was what I initially intended to do in fact. @pcc, I recall from our early internal conversations about my proposal that you felt the new mechanism should apply to all classes, so that's why the final design did that. I do tend to

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

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

[PATCH] D72386: [ThinLTO] pass UnrollLoops/VectorizeLoop/VectorizeSLP in CGOpts down to pass builder in ltobackend

tejohnson added inline comments. Herald added a subscriber: StephenFan. Comment at: lld/ELF/LTO.cpp:96 + c.PTO.LoopVectorization = c.OptLevel > 1; + c.PTO.SLPVectorization = c.OptLevel > 1; xbolva00 wrote: > COFF? This came up recently on discourse: https://

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

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 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] 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] 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] 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] 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] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

tejohnson reopened this revision. tejohnson added a comment. This revision is now accepted and ready to land. Updating with fixes since this was reverted in 80bf137fa132ea33204e98bbefa924afe9258a4e . Repository: rG LLVM Git

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

tejohnson updated this revision to Diff 251893. tejohnson added a comment. Includes fixe for 2-stage clang bootstrap test failures and an expanded fix for Chromium issue, plus new tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73242/new/ http

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

tejohnson marked 2 inline comments as done. tejohnson added a comment. PTAL Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1999 +WholeProgramDevirtResolution *Res = nullptr; +if (ExportSummary && isa(S.first.TypeID)) + // Create the type id summary reso

[PATCH] D77058: [Clang] Add llvm.loop.unroll.disable to loops with -fno-unroll-loops.

tejohnson added a comment. I think this is a good approach, rather than a per-function attribute, since as mentioned this will be preserved through inlining. @dexonsmith, does that seem reasonable to you? I missed the original patch and agree with you that we don't want to fix this in LTO by pas

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

tejohnson added a comment. FYI I have reproduced the failure, and am starting to debug. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73242/new/ https://reviews.llvm.org/D73242 ___ cfe-commits mailing

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

tejohnson added a comment. In D73242#1888295 , @tejohnson wrote: > FYI I have reproduced the failure, and am starting to debug. I see what is going on and have a simple fix, just need to create a test case. Expect to send a patch to fix this today, hope

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

tejohnson added a comment. Sent fix for review in D75201 . If the new problem showed up later and not when this one first went in, then it seems likely it is different than this issue. I'll see if I can reproduce and take a look. Repository: rG LLVM Github

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

tejohnson added a comment. In D73242#1894146 , @tejohnson wrote: > Sent fix for review in D75201 . > > If the new problem showed up later and not when this one first went in, then > it seems likely it is different than thi

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

tejohnson added a comment. The last patch from my side just went in (D74162 : [Inliner] Inlining should honor nobuiltin attributes). So I think this is now complete. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

tejohnson added a comment. Reverted in 80bf137fa132ea33204e98bbefa924afe9258a4e Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73242/new/ https://reviews.llvm.org/D73242 _

[PATCH] D71913: [LTO/WPD] Enable aggressive WPD under LTO option

tejohnson marked an inline comment as done. tejohnson added inline comments. Comment at: clang/test/CodeGenCXX/lto-visibility-inference.cpp:73 c1->f(); - // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C2" + // ITANIUM: type.test{{.*}}!"_ZTS2C2" // MS: type.test{{.*}}!"?AUC2@@" ---

[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

tejohnson created this revision. tejohnson added a reviewer: pcc. Herald added subscribers: dexonsmith, steven_wu, hiraditya, inglorion. Herald added a project: clang. Documents interaction of linker option added in D71913 with LTO visibility. Repository: rG

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

tejohnson created this revision. tejohnson added reviewers: wristow, ormris. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, steven_wu, hiraditya, inglorion, mehdi_amini. Herald added projects: clang, LLVM. There are several modifications to the optimizations performed by the Thi

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

tejohnson added a comment. This probably needs to be taken over by someone who cares about full LTO performance (@wristow or @ormris ?). This patch was some cleanup of the full LTO sample PGO pipeline, but has a number of issues I enumerate in the summary. Repository: rG LLVM Github Monorepo

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

tejohnson added a comment. In D69732#1730884 , @wenlei wrote: > > The comments indicate that this is in part due to issues with > > the new PM loop pass manager > > Wondering how different it is for these loop passes to be enabled for MonoLTO > vs ThinLT

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

tejohnson added a comment. In D61634#1682660 , @gchatelet wrote: > In D61634#1679331 , @tejohnson wrote: > > > In D61634#1635595 , @tejohnson > > wrote: > > > > > I had some

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

tejohnson marked an inline comment as done. tejohnson added a comment. In D69732#1733447 , @wristow wrote: > > This probably needs to be taken over by someone who cares about full LTO > > performance > > We at PlayStation are definitely interested in full

[PATCH] D68213: [LTO] Support for embedding bitcode section during LTO

tejohnson added a comment. It seems reasonable to support this from LTO since it provides analogous support to what is available via clang without LTO. @pcc what do you think? I only just skimmed it for now, one question below. Comment at: llvm/lib/LTO/LTOBackend.cpp:335 +

[PATCH] D68213: [LTO] Support for embedding bitcode section during LTO

tejohnson added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:335 + .Case("bitcode", EmbedBitcodeKind::Embed_Bitcode) + .Case("marker", EmbedBitcodeKind::Embed_Marker) + .Default(~0U); z

[PATCH] D68213: [LTO] Support for embedding bitcode section during LTO

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM with a few small suggestions before you commit. Comment at: llvm/lib/LTO/LTOBackend.cpp:317 +static cl::opt EmbedBitcode("lto-embed-bitcode", cl::init(false), +

[PATCH] D71193: [clang] Turn -fno-builtin flag into an IR Attribute

tejohnson added a comment. This LGTM, although I have a couple of questions that are orthogonal to this patch and shouldn't block it. Please wait to see if @aaron.ballman has any more comments. Comment at: clang/test/CodeGen/libcalls-fno-builtin.c:163 -// CHECK: [[ATTR]] =

<    1   2   3   4   5   6   7   >