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

tejohnson created this revision. tejohnson added a reviewer: wmi. Herald added subscribers: jfb, dexonsmith, steven_wu, hiraditya, inglorion, mehdi_amini. Herald added projects: clang, LLVM. I've added some more extensive ThinLTO pipeline testing with the new PM, motivated by the bug fixed in D72

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

tejohnson updated this revision to Diff 237427. tejohnson added a comment. Remove some cruft Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72538/new/ https://reviews.llvm.org/D72538 Files: clang/test/CodeGen/thinlto-distributed-newpm.ll llvm/t

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

tejohnson marked 2 inline comments as done. tejohnson added a comment. In D72538#1815074 , @wmi wrote: > The additional pipeline testing will catch any future pass change to the > pipeline. A related but separate question is do we have a way to check > w

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

tejohnson updated this revision to Diff 237462. tejohnson marked an inline comment as done. tejohnson added a comment. Implement suggestion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72538/new/ https://reviews.llvm.org/D72538 Files: clang/te

[PATCH] D72547: [llvm] Make new pass manager's OptimizationLevel a class

tejohnson added a comment. I just have a few high level comments from looking through it just now. The summary needs a fix since Os/Oz are in fact O2 so OptLevel > 1 was not doing the wrong thing. Comment at: llvm/include/llvm/Pas

[PATCH] D72523: [remark][diagnostics] Using clang diagnostic handler for IR input files

tejohnson added a comment. Thanks for fixing this missing -Rpass support! Comment at: clang/lib/CodeGen/CodeGenAction.cpp:594 + // SourceLoc. + if (Context == nullptr) { +return FullSourceLoc(); Does this only happen with IR input? Does it always happen w

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

tejohnson added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:151 + TargetMachine::initTargetOptions(M, Conf.Options); + This is going to be problematic. The Conf is a reference to the Config object saved on the LTO class instance shared by all bac

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG2af97be8027a: [ThinLTO] Add additional ThinLTO pipeline testing with new PM (authored by tejohnson). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72538/new/

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

tejohnson added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:151 + TargetMachine::initTargetOptions(M, Conf.Options); + lenary wrote: > tejohnson wrote: > > This is going to be problematic. The Conf is a reference to the Config > > object saved on

[PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

tejohnson added a comment. I'm not sure if ThinLTOCodeGenerator.cpp and LTOBackend.cpp were intentionally left out due to the LTO concerns mentioned in the description? Note if we are just passing in the Module and updating the TM based on that, it wouldn't hit the threading issue I mentioned i

[PATCH] D72523: [remark][diagnostics] Using clang diagnostic handler for IR input files

tejohnson added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:594 + // SourceLoc. + if (Context == nullptr) { +return FullSourceLoc(); xur wrote: > tejohnson wrote: > > Does this only happen with IR input? Does it always happen with IR in

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

tejohnson added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:151 + TargetMachine::initTargetOptions(M, Conf.Options); + lenary wrote: > tejohnson wrote: > > lenary wrote: > > > tejohnson wrote: > > > > This is going to be problematic. The Conf is a

[PATCH] D72523: [remark][diagnostics] Using clang diagnostic handler for IR input files

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM. One more request for a comment below that I forgot to add earlier. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:154 } +BackendConsumer(BackendAction A

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

tejohnson marked 9 inline comments as done. tejohnson added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:559 + if (!CodeGenOpts.ThinLTOIndexFile.empty()) +MPM.add(createLowerTypeTestsPass(/*ExportSummary=*/nullptr, + /*Im

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

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

[PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

tejohnson added a comment. In D72624#1820281 , @lenary wrote: > In D72624#1817464 , @tejohnson wrote: > > > > > > Thank you for your feedback! It has been very helpful. > > > I'm not sure if ThinLTOCodeGenerator.cpp

[PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

tejohnson added a comment. In D72624#1820598 , @dblaikie wrote: > (just a general comment that this code review should be only in service of > the design discussion happening on llvm-dev - please don't approve/commit > this without closing out the design

[PATCH] D72547: [llvm] Make new pass manager's OptimizationLevel a class

tejohnson added a comment. Overall I like this approach. Comment at: llvm/lib/Passes/PassBuilder.cpp:246 -static bool isOptimizingForSize(PassBuilder::OptimizationLevel Level) { - switch (Level) { - case PassBuilder::O0: - case PassBuilder::O1: - case PassBuilder::O2: -

[PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

tejohnson added a comment. From an LTO perspective, this seems fine for the reasons we discussed here. I looked through the patch and have a few comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:818 + if (TM) { +TM->initializeOptionsWithModuleMetadata(*TheModule);

[PATCH] D72547: [llvm] Make new pass manager's OptimizationLevel a class

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM One thing to consider changing/removing in the summary is this comment: "For example, (enum) "Level > 1" captures not only O2 and O3

[PATCH] D87622: [MemProf] Rename HeapProfiler to MemProfiler for consistency (NFC)

tejohnson created this revision. tejohnson added a reviewer: davidxl. Herald added subscribers: jfb, hiraditya, mgorny. Herald added projects: clang, LLVM. tejohnson requested review of this revision. This is consistent with the clang option added in 7ed8124d46f94601d5f1364becee9cee8538265e

[PATCH] D87622: [MemProf] Rename HeapProfiler to MemProfiler for consistency (NFC)

tejohnson added a comment. In D87622#2271861 , @MaskRay wrote: > LG. (Nit: technically this is not NFC: it affects command line options and > runtime function names (ABI changes).) Fair enough, will remove that. Repository: rG LLVM Github Monorepo C

[PATCH] D87622: [MemProf] Rename HeapProfiler to MemProfiler for consistency

tejohnson added a comment. In D87622#2271931 , @davidxl wrote: > LGTM (if the option is documented, the documentation part also needs to be > updated). It isn't documented yet, since the generated code won't work until the runtime is in. I will be send

[PATCH] D87622: [MemProf] Rename HeapProfiler to MemProfiler for consistency

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG226d80ebe20e: [MemProf] Rename HeapProfiler to MemProfiler for consistency (authored by tejohnson). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes)

tejohnson added a comment. In D86958#2274040 , @hans wrote: > Please go ahead and commit to the branch. Sorry, I thought I already did! Just pushed the commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D87636: [ThinLTO] add post-thinlto-merge option to -lto-embed-bitcode

tejohnson added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:621 +"command line arguments are not available"); +llvm::EmbedBitcodeInModule(Mod, llvm::MemoryBufferRef(), + /*EmbedBitcode*/ true, How

[PATCH] D87636: [ThinLTO] add post-thinlto-merge option to -lto-embed-bitcode

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM, just needs a comment update as noted below and also an update to the patch title. Comment at: llvm/lib/LTO/LTOBackend.cpp:368 + LLVM_DEBUG( + dbgs(

[PATCH] D87943: [clang] Remove profile availabile check for fsplit-machine-functions.

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm with a couple of minor suggestions Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4919 if (Triple.isX86() && Triple.isOSBinFormatELF()) { if (A->get

[PATCH] D87949: [ThinLTO] Option to bypass function importing.

tejohnson added a comment. Minor comments. Comment at: clang/test/CodeGen/thinlto_embed_bitcode.ll:17 +; RUN: rm %t1.bc %t2.bc +; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t-redo.o -x ir %t-embedded.bc -c -fthinlto-index=%t.o.thinlto.bc -mllvm -lto-embed-bitcode=po

[PATCH] D87949: [ThinLTO] Option to bypass function importing.

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm with comment typo fix Comment at: llvm/include/llvm/LTO/LTOBackend.h:59 + +/// Distributed ThinlTO: load the referenced modules, keeping their buffers +/// alive i

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

tejohnson added a comment. In D77925#2229484 , @mehdi_amini wrote: > Overall that would likely work for XLA. Something I'd like to mention though > in response to: > >> The veclib type is also tied to the accepted values for -fveclib, which is a >> list

[PATCH] D85810: [clang] Pass-through remarks options to linker

tejohnson added a comment. Looks pretty good but I think it can be simplified in one place as I note below. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:596 + // Handle remark diagnostics on screen options: '-Rpass-*'. + if (usesRpassOptions(Args)) +renderRpassO

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

tejohnson added inline comments. Comment at: llvm/lib/Analysis/OptimizationRemarkEmitter.cpp:103 BFI = &getAnalysis().getBFI(); - else +// Get hotness threshold from PSI. This should only happen once. +if (Context.isDiagnosticsHotnessThresholdSetFromPSI()) { ---

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

tejohnson added a comment. @pcc ping. I'd like to get something in for LLVM 11 if at all possible. How about for now I make the last sentence something like "This is useful in situations where it is not safe to specify``-fvisibility=hidden`` at compile time, for example when bitcode objects wil

[PATCH] D83845: [LTO/WPD] Remove special type test handling for -flto-visibility-public-std

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

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

tejohnson marked an inline comment as done. tejohnson added a comment. In D75655#2237474 , @pcc wrote: > I think that the part of the description beginning "This can be used when..." > is somewhat misleading since you could pretty much say the same thing

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

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

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG72bdb41a06a2: [Docs] Document --lto-whole-program-visibility (authored by tejohnson). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75655/new/ https://revie

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

tejohnson marked 18 inline comments as done. tejohnson added a comment. I think I've addressed all the comments. Comment at: llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp:48 +// Size of memory mapped to a single shadow location. +static const uint64_t DefaultShadowGranul

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

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

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

tejohnson added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp:563 +return false; + if (!ClDebugFunc.empty() && ClDebugFunc == F.getName()) +return false; MaskRay wrote: > `ClDebugFunc != F.getName()` Your suggestion do

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

tejohnson updated this revision to Diff 288191. tejohnson added a comment. Remove unnecessary check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85948/new/ https://reviews.llvm.org/D85948 Files: clang/include/clang/Basic/CodeGenOptions.def cl

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7ed8124d46f9: [HeapProf] Clang and LLVM support for heap profiling instrumentation (authored by tejohnson). Repository: rG LLVM Github Monorepo C

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

tejohnson added a comment. In D85948#2242352 , @lebedev.ri wrote: > This really needs some docs changes. Good point, I'll send a patch for that later today. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D859

[PATCH] D85948: [HeapProf] Clang and LLVM support for heap profiling instrumentation

tejohnson added a comment. In D85948#2242370 , @tejohnson wrote: > In D85948#2242352 , @lebedev.ri > wrote: > >> This really needs some docs changes. > > Good point, I'll send a patch for that later today. Actuall

[PATCH] D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes)

tejohnson created this revision. tejohnson added a reviewer: hans. Herald added subscribers: dexonsmith, inglorion. Herald added a reviewer: MaskRay. Herald added projects: clang, LLVM. tejohnson requested review of this revision. This adds documentation for the options added / changed by D71913

[PATCH] D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes)

tejohnson updated this revision to Diff 289238. tejohnson added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86958/new/ https://reviews.llvm.org/D86958 Files: clang/docs/ReleaseNotes.rst lld/docs/ReleaseNotes.rst

[PATCH] D73418: [WPD] Emit vcall_visibility metadata for MicrosoftCXXABI

tejohnson added a comment. In D73418#2461943 , @ayermolo wrote: > @tejohnson I was wondering where does this @anon.{{.*}} gets set, or in > general where can I find more information about it. > > When I step through the code in emitVTableTypeMetadata, it'

[PATCH] D91410: [llvm][clang][mlir] Add checks for the return values from Target::createXXX to prevent protential null deref

tejohnson added a comment. I was reviewing the llvm/lib/* changes and note that the new checking is a mix of assert (debug or assert enabled compiler only) and fatal errors (both debug and release compilers). Is that intended? If these are not likely due to user input issues, then perhaps they

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

tejohnson added a comment. Largely LGTM, except that the clang changes are missing a corresponding test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85808/new/ https://reviews.llvm.org/D85808 ___ cfe-c

[PATCH] D91585: [NPM] Move more O0 pass building into PassBuilder

tejohnson added a comment. Thanks, generally this seems to be good cleanup. Question on one of the changes below though. Comment at: llvm/lib/Passes/PassBuilder.cpp:2365 + // Don't do anything for (thin)lto backend compiles at O0. + if (Matches[1] != "thinlto" && Mat

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

tejohnson created this revision. tejohnson added reviewers: pcc, ostannard. Herald added a project: clang. tejohnson requested review of this revision. Add a Visited set to avoid repeatedly processing the same base classes in complex class hierarchies. This cut down the compile time of one source

[PATCH] D91410: [llvm][clang][mlir] Add checks for the return values from Target::createXXX to prevent protential null deref

tejohnson added a comment. In D91410#2401751 , @OikawaKirie wrote: > In D91410#2400018 , @tejohnson wrote: > >> > > > >> ... the new checking is a mix of assert and fatal errors. Is that intended? > > No. The added

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

tejohnson added a comment. Thanks for adding the Driver test. I was thinking of something to test the CompilerInvocation changes, similar to your test using opt, that ensures the option has the desired behavior when invoked via clang. Looks like there is an existing test clang/test/Frontend/opt

[PATCH] D91585: [NPM] Move more O0 pass building into PassBuilder

tejohnson added inline comments. Comment at: llvm/lib/Passes/PassBuilder.cpp:2365 +Matches[1] != "lto") { + MPM.addPass(buildO0DefaultPipeline(L, Matches[1] != "default")); return Error::success(); I think it would be clearer to check against

[PATCH] D91585: [NPM] Move more O0 pass building into PassBuilder

tejohnson added inline comments. Comment at: llvm/lib/Passes/PassBuilder.cpp:2365 +Matches[1] != "lto") { + MPM.addPass(buildO0DefaultPipeline(L, Matches[1] != "default")); return Error::success(); tejohnson wrote: > I think it would be cleare

[PATCH] D91585: [NPM] Move more O0 pass building into PassBuilder

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/D91585/new/ https://reviews.llvm.org/D91585 ___

[PATCH] D91812: [ThinLTO/WPD] Enable -wholeprogramdevirt-skip in ThinLTO backends

tejohnson created this revision. tejohnson added reviewers: pcc, evgeny777. Herald added subscribers: cfe-commits, steven_wu, hiraditya, inglorion, Prazek. Herald added projects: clang, LLVM. tejohnson requested review of this revision. Previously this option could be used to skip devirtualization

[PATCH] D91410: [llvm][clang][mlir] Add checks for the return values from Target::createXXX to prevent protential null deref

tejohnson accepted this revision. tejohnson added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91410/new/ https://reviews.llvm.org/D91410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

tejohnson added a comment. Can you add a test? You can probably just beef up clang/test/CodeGenCXX/virtual-function-elimination.cpp to test this option as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88349/new/ https://reviews.llvm.org/D883

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM with the test change suggested below. Comment at: clang/test/CodeGenCXX/virtual-function-elimination.cpp:2 // RUN: %clang_cc1 -triple x86_64-unknown-linux -flto -

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

tejohnson accepted this revision. tejohnson added a comment. LGTM for test - thanks! Yeah, it looks like that cleanup patch inadvertently fixed this issue as a side effect. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88349/new/ https://reviews.l

[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

tejohnson created this revision. tejohnson added a reviewer: davidxl. Herald added subscribers: wenlei, dang, hiraditya. Herald added projects: clang, LLVM. tejohnson requested review of this revision. Similar to -fprofile-generate=, add -fmemory-profile= which takes a directory path. This is pass

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

tejohnson added a comment. From a ThinLTO perspective, no specific concerns as the buildModuleSimplificationPipeline is invoked in both the pre and post LTO link pipelines, so they both get an equivalent change. But there is an issue for regular LTO, noted below. Comment at:

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

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

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

tejohnson added a comment. Adding another reviewer - @wmi can you take a look? This is a straightforward compile time fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91676/new/ https://reviews.llvm.org/D91676 __

[PATCH] D91812: [ThinLTO/WPD] Enable -wholeprogramdevirt-skip in ThinLTO backends

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6e4c1cf29388: [ThinLTO/WPD] Enable -wholeprogramdevirt-skip in ThinLTO backends (authored by tejohnson). Repository: rG LLVM Github Monorepo CHAN

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

tejohnson added inline comments. Comment at: clang/lib/CodeGen/CGVTables.cpp:1301-1302 + // has no effect on the min visibility computed below by the recursive caller. + if (!Visited.insert(RD).second) +return llvm::GlobalObject::VCallVisibilityTranslationUnit; + -

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

tejohnson updated this revision to Diff 307404. tejohnson added a comment. Improve comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91676/new/ https://reviews.llvm.org/D91676 Files: clang/lib/CodeGen/CGVTables.cpp clang/lib/CodeGen/CodeGe

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

tejohnson added inline comments. Comment at: clang/lib/CodeGen/CGVTables.cpp:1301-1302 + // has no effect on the min visibility computed below by the recursive caller. + if (!Visited.insert(RD).second) +return llvm::GlobalObject::VCallVisibilityTranslationUnit; + -

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

This revision was automatically updated to reflect the committed changes. Closed by commit rG0768b0576a93: Avoid redundant work when computing vtable vcall visibility (authored by tejohnson). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91676/new/

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm with a couple of minor nits noted below that you can fix before submitting Comment at: clang/include/clang/Basic/CodeGenOptions.h:353 + /// The threshold is an Op

[PATCH] D92673: [ThinLTO] Remove unused symbol declarations when possible

tejohnson created this revision. tejohnson added a reviewer: wmi. Herald added subscribers: wenlei, arphaman, steven_wu, hiraditya, inglorion, Prazek. tejohnson requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits. Follow on to D42816

[PATCH] D92673: [ThinLTO] Remove unused symbol declarations when possible

tejohnson abandoned this revision. tejohnson added a comment. Subsumed by D92804 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92673/new/ https://reviews.llvm.org/D92673 ___

[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

tejohnson updated this revision to Diff 298849. tejohnson added a comment. Herald added a project: Sanitizers. Herald added a subscriber: Sanitizers. As discussed in review for D89086 , move test changes to this patch since they are only required here. Repositor

[PATCH] D90278: [ThinLTO] Fix .llvmcmd emission

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM but is there a functional reason why CmdArgs was changed to be passed by reference? If just generic cleanup might be better to split up that into a separate commit. Repository:

[PATCH] D90330: [NFC][ThinLTO] Change command line passing to EmbedBitcodeInModule

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm except for the part that should go back into D90278 as noted below. Comment at: llvm/include/llvm/Bitcode/BitcodeWriter.h:165

[PATCH] D90278: [ThinLTO] Fix .llvmcmd emission

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

[PATCH] D90366: [ThinLTO] Strenghten the test for .llvmcmd embedding

tejohnson added a comment. The motivation and effects of the change are unclear to me. Does this mean that the .llvmcmd section is always emitted? Or always emitted whenever any bitcode embedding is requested? Comment at: clang/test/CodeGen/thinlto_embed_bitcode.ll:28 ; RUN:

[PATCH] D90366: [ThinLTO] Fix empty .llvmcmd sections

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/D90366/new/ https://reviews.llvm.org/D90366 ___

[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

tejohnson added a comment. In D89087#2359962 , @davidxl wrote: > There should be a related LLVM side of changes. Is it in a different patch? That's here too. See the change in llvm/lib/Transforms/Instrumentation/MemProfiler.cpp and a related test (they

[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

tejohnson updated this revision to Diff 302112. tejohnson added a comment. Rebase and fix bad merge Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89087/new/ https://reviews.llvm.org/D89087 Files: clang/include/clang/Basic/CodeGenOptions.def cl

[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

tejohnson marked 3 inline comments as done. tejohnson added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1049 + } else if (Args.hasArg(OPT_fmemory_profile)) +Opts.MemoryProfileOutput = MemProfileBasename; MaskRay wrote: > std::str

[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

tejohnson updated this revision to Diff 302119. tejohnson added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89087/new/ https://reviews.llvm.org/D89087 Files: clang/include/clang/Basic/CodeGenOptions.def clang/incl

[PATCH] D89087: [MemProf] Pass down memory profile name with optional path from clang

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0949f96dc652: [MemProf] Pass down memory profile name with optional path from clang (authored by tejohnson). Repository: rG LLVM Github Monorepo

[PATCH] D99683: [HIP] Support ThinLTO

tejohnson added a comment. In D99683#2744764 , @yaxunl wrote: > In D99683#2683308 , @tejohnson wrote: > >> To do what I suggested in the prior comment, you'd probably want to add a >> new index-wide flag (since we

[PATCH] D99683: [HIP] Support ThinLTO

tejohnson added inline comments. Comment at: llvm/lib/Transforms/IPO/FunctionImport.cpp:496 dbgs() << "ignored! No qualifying callee with summary found.\n"); continue; } yaxunl wrote: > tejohnson wrote: > > Probably better to issue an

[PATCH] D99683: [HIP] Support ThinLTO

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99683/new/ https://reviews.llvm.org/D99683 ___ cfe-commits mailing list cfe-commi

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

tejohnson added a comment. In D102742#2767569 , @nickdesaulniers wrote: > Obviously needs work/cleanup, changes to x86, and tests, but posting for > early feedback about module level attributes vs function level attributes, or > possibly something else

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

tejohnson added a comment. In D102742#2774185 , @nickdesaulniers wrote: > - upgrade module merge strategy to Error Probably want a test using llvm-link or llvm-lto to check this behavior (that alike flags are getting propagated as expected and that con

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

tejohnson added a comment. In D102742#2774639 , @nickdesaulniers wrote: > In D102742#2774562 , @tejohnson > wrote: > >> In D102742#2774185 , >> @nickdesaulniers wrote:

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

tejohnson added inline comments. Comment at: clang/include/clang/Driver/Options.td:3429 HelpText<"Use the given reg for addressing the stack-protector guard">, - MarshallingInfoString, [{"none"}]>; + MarshallingInfoString>; def mfentry : Flag<["-"], "mfentry">, HelpText<"In

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

tejohnson added inline comments. Comment at: clang/include/clang/Driver/Options.td:3429 HelpText<"Use the given reg for addressing the stack-protector guard">, - MarshallingInfoString, [{"none"}]>; + MarshallingInfoString>; def mfentry : Flag<["-"], "mfentry">, HelpText<"In

[PATCH] D102742: [IR] make stack-protector-guard-* flags into module attrs

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/D102742/new/ https://reviews.llvm.org/D102742 _

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

tejohnson created this revision. tejohnson added reviewers: yaxunl, tra. Herald added a subscriber: inglorion. tejohnson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. A recent change (D99683 ) to support T

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

tejohnson added inline comments. Comment at: clang/test/Driver/hip-options.hip:72 +// HIPTHINLTO-NOT: error: invalid argument '-fwhole-program-vtables' only allowed with '-flto' +// HIPTHINLTO-NOT: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-flto-unit" +// HIPTHIN

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

tejohnson added a comment. Herald added a subscriber: ormris. > Curiously, using ModFlagBehavior::Error doesn't error if one of two modules > being linked together doesn't have such a module level attribute. Yeah, there's a Require behavior, but that only allows you to specify what the value sh

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

tejohnson added a comment. Ok thanks. I need to go through the propagation code and tests again more closely now, but one question/suggestion below in the meantime. Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:580 +// If there are calls to unknown targets (e.g. in

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

tejohnson added inline comments. Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:580 +// If there are calls to unknown targets (e.g. indirect) +unsigned hasUnknownCall : 1; + modimo wrote: > tejohnson wrote: > > Now that we have MayThrow, can we avo

[PATCH] D109956: Fix CodeGen/pgo-sample-thinlto-summary.c with old PM

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. lgtm. And I just was informed in a recent review of one of my patches that we are now removing old PM tests, so if you want you could also go ahead and remove the first 2 lines here. R

<    1   2   3   4   5   6   7   >