[PATCH] D29067: IRGen: When loading the main module in the distributed ThinLTO backend, look for the module containing the summary.

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D29067 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29067: IRGen: When loading the main module in the distributed ThinLTO backend, look for the module containing the summary.

tejohnson added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:867 +Expected clang::FindThinLTOModule(MemoryBufferRef MBRef) { + Expected BMsOrErr = getBitcodeModuleList(MBRef); Would it be better to have this in llvm (e.g. in

[PATCH] D28843: IRGen: Start using the WriteThinLTOBitcode pass.

tejohnson added a comment. > TODO: avoid breaking Darwin. How does this break Darwin? Comment at: clang/lib/CodeGen/BackendUtil.cpp:694 +else + PerModulePasses.add( + createBitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists)); Can we

[PATCH] D28843: IRGen: Start using the WriteThinLTOBitcode pass.

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D28843 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D28746: Mention ThinLTO with PGO in ReleaseNotes 4.0

tejohnson added a comment. In https://reviews.llvm.org/D28746#646793, @Prazek wrote: > Maybe I should change ThinLTO for '-flto=thin' and also mention this > improvement in LLVM release notes? > > BTW '-flto=thin' is not documented in UsersManual It's documented here:

[PATCH] D28843: IRGen: Start using the WriteThinLTOBitcode pass.

tejohnson added inline comments. Comment at: clang/test/CodeGenCXX/type-metadata-thinlto.cpp:2 +// RUN: %clang_cc1 -flto=thin -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s +// RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s +

[PATCH] D28843: IRGen: Start using the WriteThinLTOBitcode pass.

tejohnson added inline comments. Comment at: clang/test/CodeGenCXX/type-metadata-thinlto.cpp:2 +// RUN: %clang_cc1 -flto=thin -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s +// RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s +

[PATCH] D28588: Pass -fprofile-sample-use to lto backends.

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D28588 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D28588: Pass -fprofile-sample-use to lto backends.

tejohnson added a comment. In https://reviews.llvm.org/D28588#644637, @danielcdh wrote: > Looks like this is still breaking these buildbots: > > http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/3216/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Athinlto_backend.ll > > I reverted

[PATCH] D28588: Pass -fprofile-sample-use to lto backends.

tejohnson added a comment. In https://reviews.llvm.org/D28588#644472, @tejohnson wrote: > Otherwise I think "; REQUIRES: asserts" might do the trick? Looks like it - I looked at another test that used -debug output and it requires asserts. https://reviews.llvm.org/D28588

[PATCH] D28588: Pass -fprofile-sample-use to lto backends.

tejohnson added a comment. In https://reviews.llvm.org/D28588#644489, @danielcdh wrote: > Thanks for the prompt response. > > But looks like several other tests also has "-mllvm -debug-pass=Structure" in > their tests: > > tools/clang/test/CodeGen/pgo-instrumentation.c >

[PATCH] D28588: Pass -fprofile-sample-use to lto backends.

tejohnson added a comment. In https://reviews.llvm.org/D28588#644467, @danielcdh wrote: > The breaks some buildbots thus I reverted the patch: > > http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/1889 > >

[PATCH] D28362: [ThinLTO] Optionally ignore empty index file

tejohnson created this revision. tejohnson added a reviewer: mehdi_amini. tejohnson added a subscriber: cfe-commits. In order to simplify distributed build system integration, where actions may be scheduled before the Thin Link which determines the list of objects selected by the linker. The gold

[PATCH] D28362: [ThinLTO] Optionally ignore empty index file

tejohnson added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:65 +"Ignore an empty index file and perform non-ThinLTO compilation"), +llvm::cl::init(false)); + tejohnson wrote: > mehdi_amini wrote: > > Is it common to do this in clang? >

[PATCH] D28362: [ThinLTO] Optionally ignore empty index file

tejohnson updated this revision to Diff 83405. tejohnson added a comment. Move option to LLVM. https://reviews.llvm.org/D28362 Files: lib/CodeGen/BackendUtil.cpp test/CodeGen/thinlto_backend.ll Index: test/CodeGen/thinlto_backend.ll

[PATCH] D28362: [ThinLTO] Optionally ignore empty index file

tejohnson added a comment. In https://reviews.llvm.org/D28362#638260, @mehdi_amini wrote: > > In order to simplify distributed build system integration, where actions > > may be scheduled before the Thin Link which determines the list of > > objects selected by the linker. The gold plugin

[PATCH] D28362: [ThinLTO] Optionally ignore empty index file

tejohnson added a comment. In https://reviews.llvm.org/D28362#638203, @mehdi_amini wrote: > LGTM Thanks, let me know if the llvm companion patch (https://reviews.llvm.org/D28410) is ok as well. https://reviews.llvm.org/D28362 ___ cfe-commits

[PATCH] D28362: [ThinLTO] Optionally ignore empty index file

This revision was automatically updated to reflect the committed changes. Closed by commit rL291303: [ThinLTO] Optionally ignore empty index file (authored by tejohnson). Changed prior to commit: https://reviews.llvm.org/D28362?vs=83405=83450#toc Repository: rL LLVM

[PATCH] D28139: [ThinLTO] No need to rediscover imports in distributed backend

tejohnson created this revision. tejohnson added reviewers: mehdi_amini, pcc. tejohnson added a subscriber: cfe-commits. We can simply import all external values with summaries included in the individual index file created for the distributed backend job, as only those are added to the individual

[PATCH] D28139: [ThinLTO] No need to rediscover imports in distributed backend

This revision was automatically updated to reflect the committed changes. Closed by commit rL290674: [ThinLTO] No need to rediscover imports in distributed backend (authored by tejohnson). Changed prior to commit: https://reviews.llvm.org/D28139?vs=82603=82608#toc Repository: rL LLVM

[PATCH] D31219: Update the SamplePGO test to verify that unroll/icp is not invoked in thinlto compile phase.

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM but please add comments about what we're testing. https://reviews.llvm.org/D31219 ___ cfe-commits mailing list

[PATCH] D31050: [ThinLTO] Clang support for emitting minimized bitcode for thin link

This revision was automatically updated to reflect the committed changes. Closed by commit rL298639: [ThinLTO] Clang support for emitting minimized bitcode for thin link (authored by tejohnson). Changed prior to commit: https://reviews.llvm.org/D31050?vs=92647=92849#toc Repository: rL LLVM

[PATCH] D31050: [ThinLTO] Clang support for emitting minimized bitcode for thin link

tejohnson updated this revision to Diff 92647. tejohnson added a comment. Update as per review comments on llvm side patch https://reviews.llvm.org/D31027. https://reviews.llvm.org/D31050 Files: include/clang/Driver/CC1Options.td include/clang/Frontend/CodeGenOptions.h

[PATCH] D31114: Refactor `initTargetOptions` out of `EmitAssemblyHelper::CreateTargetMachine` and use it to initialize TargetOptions for ThinLTO Backends

tejohnson added a comment. Ping - I think this just needs a test in addition to undoing the AsmHelper definition move. I have some follow on changes I want to send that set up the rest of the lto::Config fields (e.g. the RelocModel etc). https://reviews.llvm.org/D31114

[PATCH] D31101: [ThinLTO] Use clang's existing code gen handling for ThinLTO backends

tejohnson added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:1007 else AsmHelper.EmitAssembly(Action, std::move(OS)); tejohnson wrote: > I just noticed that EmitAssembly does a lot more than just emission - it is > also setting up an

[PATCH] D31101: [ThinLTO] Use clang's existing code gen handling for ThinLTO backends

tejohnson updated this revision to Diff 93535. tejohnson added a comment. Use LTO to emit LLVM IR https://reviews.llvm.org/D31101 Files: lib/CodeGen/BackendUtil.cpp test/CodeGen/thinlto-emit-llvm.c Index: test/CodeGen/thinlto-emit-llvm.c

[PATCH] D31114: Refactor `initTargetOptions` out of `EmitAssemblyHelper::CreateTargetMachine` and use it to initialize TargetOptions for ThinLTO Backends

tejohnson added a comment. As discussed with Mehdi offline, I am taking this one over. I just mailed https://reviews.llvm.org/D31508 which supersedes this one. https://reviews.llvm.org/D31114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D31508: [ThinLTO] Set up lto::Config properly for codegen in ThinLTO backends

tejohnson created this revision. Herald added a subscriber: Prazek. This involved refactoring out pieces of EmitAssemblyHelper::CreateTargetMachine for use in runThinLTOBackend. Subsumes https://reviews.llvm.org/D31114. https://reviews.llvm.org/D31508 Files: lib/CodeGen/BackendUtil.cpp

[PATCH] D31534: [ThinLTO] Handle -emit-llvm* in ThinLTO backends

tejohnson updated this revision to Diff 93695. tejohnson added a comment. Implement review suggestions https://reviews.llvm.org/D31534 Files: lib/CodeGen/BackendUtil.cpp test/CodeGen/thinlto-emit-llvm.c Index: test/CodeGen/thinlto-emit-llvm.c

[PATCH] D31114: Refactor `initTargetOptions` out of `EmitAssemblyHelper::CreateTargetMachine` and use it to initialize TargetOptions for ThinLTO Backends

tejohnson added a comment. I think you won't get the correct handling of -emit-llvm and -emit-llvm-bc since we don't get the handling for Backend_Emit* in EmitAssemblyHelper::EmitAssembly. Comment at: clang/lib/CodeGen/BackendUtil.cpp:595 llvm::Optional RM; RM =

[PATCH] D31101: [ThinLTO] Use clang's existing code gen handling for ThinLTO backends

tejohnson created this revision. Herald added subscribers: Prazek, mehdi_amini. We noticed that when invoking the thinBackend via clang (for the distributed build case) that flags like -ffunction-sections and -emit-llvm were not having the intended effect. This could have been fixed by setting up

[PATCH] D31114: Refactor `initTargetOptions` out of `EmitAssemblyHelper::CreateTargetMachine` and use it to initialize TargetOptions for ThinLTO Backends

tejohnson added a comment. In https://reviews.llvm.org/D31114#704726, @mehdi_amini wrote: > In https://reviews.llvm.org/D31114#704649, @tejohnson wrote: > > > I think you won't get the correct handling of -emit-llvm and -emit-llvm-bc > > since we don't get the handling for Backend_Emit* in > >

[PATCH] D31114: Refactor `initTargetOptions` out of `EmitAssemblyHelper::CreateTargetMachine` and use it to initialize TargetOptions for ThinLTO Backends

tejohnson added a comment. In https://reviews.llvm.org/D31114#704733, @mehdi_amini wrote: > In https://reviews.llvm.org/D31114#704728, @tejohnson wrote: > > > In https://reviews.llvm.org/D31114#704726, @mehdi_amini wrote: > > > > > In https://reviews.llvm.org/D31114#704649, @tejohnson wrote: > >

[PATCH] D31101: [ThinLTO] Use clang's existing code gen handling for ThinLTO backends

tejohnson updated this revision to Diff 92324. tejohnson added a comment. As discussed in the review threads for https://reviews.llvm.org/D31114 and https://reviews.llvm.org/D31100, only fall back to clang's output file emission for -emit-llvm and -emit-llvm-bc. https://reviews.llvm.org/D31101

[PATCH] D31114: Refactor `initTargetOptions` out of `EmitAssemblyHelper::CreateTargetMachine` and use it to initialize TargetOptions for ThinLTO Backends

tejohnson added a comment. In https://reviews.llvm.org/D31114#704749, @mehdi_amini wrote: > In https://reviews.llvm.org/D31114#704748, @tejohnson wrote: > > > In https://reviews.llvm.org/D31114#704733, @mehdi_amini wrote: > > > > > In https://reviews.llvm.org/D31114#704728, @tejohnson wrote: > >

[PATCH] D31101: [ThinLTO] Use clang's existing code gen handling for ThinLTO backends

tejohnson added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:1007 else AsmHelper.EmitAssembly(Action, std::move(OS)); I just noticed that EmitAssembly does a lot more than just emission - it is also setting up an optimization pipeline in

[PATCH] D31050: [ThinLTO] Clang support for emitting minimized bitcode for thin link

tejohnson created this revision. Herald added a subscriber: Prazek. Clang companion patch to LLVM patch https://reviews.llvm.org/D31027, which adds support for emitting minimized bitcode file for use in the thin link step. Add a cc1 option -fthin-link-bitcode= to trigger this behavior. Depends

[PATCH] D31202: Clang change: Do not inline hot callsites for samplepgo in thinlto compile phase.

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D31202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D31508: [ThinLTO] Set up lto::Config properly for codegen in ThinLTO backends

tejohnson added a comment. In https://reviews.llvm.org/D31508#716087, @davide wrote: > clang -cc1 crashes if an invalid reloc model is passed (via > `-mreloc-model=`), see https://bugs.llvm.org/show_bug.cgi?id=32490 > I'm not sure if the bug was already there or this refactoring exposed it. >

[PATCH] D31534: [ThinLTO] Handle -emit-llvm* in ThinLTO backends

This revision was automatically updated to reflect the committed changes. Closed by commit rL299274: [ThinLTO] Handle -emit-llvm* in ThinLTO backends (authored by tejohnson). Changed prior to commit: https://reviews.llvm.org/D31534?vs=93695=93722#toc Repository: rL LLVM

[PATCH] D31508: [ThinLTO] Set up lto::Config properly for codegen in ThinLTO backends

This revision was automatically updated to reflect the committed changes. Closed by commit rL299152: [ThinLTO] Set up lto::Config properly for codegen in ThinLTO backends (authored by tejohnson). Changed prior to commit: https://reviews.llvm.org/D31508?vs=93536=93577#toc Repository: rL LLVM

[PATCH] D30792: Use callback for internalizing linked symbols.

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM with the format fix I pointed out. Comment at: lib/CodeGen/CodeGenAction.cpp:186 +} else { + Err = Linker::linkModules(*getModule(),

[PATCH] D30792: Use callback for internalizing linked symbols.

tejohnson added a comment. Great, thanks. One question below and a format nit. Comment at: lib/CodeGen/CodeGenAction.cpp:177 +if (LM.Internalize) { + Err = Linker::linkModules( + *getModule(), std::move(LM.Module), LM.LinkFlags,

[PATCH] D30920: Do not pass -Os and -Oz to the Gold plugin

tejohnson added a comment. In https://reviews.llvm.org/D30920#700133, @pcc wrote: > In https://reviews.llvm.org/D30920#700077, @tejohnson wrote: > > > Until everything is converted to using size attributes, it seems like a > > correct fix for the bug is to accept these options in the

[PATCH] D30920: Do not pass -Os and -Oz to the Gold plugin

tejohnson added a comment. Interested in pcc's thoughts, as https://bugs.llvm.org/show_bug.cgi?id=32155 mentioned you already discussed with him. Note that some of the passes that check PassManagerBuilder::sizeLevel are added during the ThinLTO back end (e.g. populateModulePassManager which

[PATCH] D31534: [ThinLTO] Handle -emit-llvm* in ThinLTO backends

tejohnson created this revision. Herald added a subscriber: Prazek. Use PreCodeGenModuleHook to invoke the correct writer when emitting LLVM IR, returning false to skip codegen from within thinBackend. https://reviews.llvm.org/D31534 Files: lib/CodeGen/BackendUtil.cpp

[PATCH] D31101: [ThinLTO] Use clang's existing code gen handling for ThinLTO backends

tejohnson abandoned this revision. tejohnson added a comment. Subsumed by https://reviews.llvm.org/D31534 https://reviews.llvm.org/D31101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35081: [ThinLTO] Allow multiple summary entries.

tejohnson added a comment. In https://reviews.llvm.org/D35081#808158, @mehdi_amini wrote: > Thanks, very clear :) > > I would expect that if we reprocess a GUID we invalidate the previous import > of the same GUID. Right now my impression is that the issue is that ` >

[PATCH] D35081: [ThinLTO] Allow multiple summary entries.

tejohnson added a comment. In https://reviews.llvm.org/D35081#808124, @mehdi_amini wrote: > Teresa: can you describe a bit more how we end up importing two summaries for > the same GUID? I would expect that after importing one, we don't even come to > try to import another since we already

[PATCH] D35081: [ThinLTO] Allow multiple summary entries.

tejohnson added a comment. In https://reviews.llvm.org/D35081#808204, @mehdi_amini wrote: > In https://reviews.llvm.org/D35081#808189, @tejohnson wrote: > > > > Alternatively (and likely preferably from a compile-time point of view to > > > limit the list of import), we could keep a map of

[PATCH] D35081: [ThinLTO] Allow multiple summary entries.

tejohnson added a comment. In https://reviews.llvm.org/D35081#807497, @mehdi_amini wrote: > In https://reviews.llvm.org/D35081#807172, @fhahn wrote: > > > It's @yunlian, so if the issue you described above covers his failure, than > > that's great. @tejohnson do you have time to work on a fix

[PATCH] D35081: [ThinLTO] Allow multiple summary entries.

tejohnson added a comment. In https://reviews.llvm.org/D35081#808517, @mehdi_amini wrote: > In https://reviews.llvm.org/D35081#808207, @tejohnson wrote: > > > My first option was if any copy is under the threshold, simply pick the > > prevailing copy (it may be over threshold, but assume we

[PATCH] D35153: Use DenseMap instead std::map for GVSummaryMapTy

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D35153 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35081: [ThinLTO] Allow multiple summary entries.

tejohnson added a comment. In https://reviews.llvm.org/D35081#805127, @fhahn wrote: > To reproduce the issue you could use > > +; Check that we only add a single summary entry for multiple definitions > +; of a linkonce_odr function > + > +; RUN: opt -module-summary %s -o %t1.bc > +;

[PATCH] D35081: [ThinLTO] Allow multiple summary entries.

tejohnson added a comment. In https://reviews.llvm.org/D35081#805506, @yunlian wrote: > This error happens when I try to triage a thinLTO failure on ARM. > > Initially I got some error like > Instruction does not dominate all uses! > > %205 = bitcast i1 (%"class.blink::LayoutObject"*)** %194

[PATCH] D35081: [ThinLTO] Allow multiple summary entries.

tejohnson added a comment. How are you invoking this? Typically this path is invoked for distributed builds, when the individual index files for each backend were written during the thin link (e.g. via gold plugin's -thinlto-index-only option). In that case, we only expect a single summary to

[PATCH] D35081: [ThinLTO] Allow multiple summary entries.

tejohnson added a comment. In https://reviews.llvm.org/D35081#808789, @tejohnson wrote: > In https://reviews.llvm.org/D35081#808517, @mehdi_amini wrote: > > > In https://reviews.llvm.org/D35081#808207, @tejohnson wrote: > > > > > My first option was if any copy is under the threshold, simply

[PATCH] D35081: [ThinLTO] Allow multiple summary entries.

tejohnson added a comment. In https://reviews.llvm.org/D35081#805761, @yunlian wrote: > I've sent a reproduce test case to tejohnson. FYI I tracked down what is going on here and reproduced with a small test case. Essentially, two copies of a linkonce odr function were optimized somewhat

[PATCH] D34790: [NewPM] Add Clang cc1 flag -fdebug-pass-manager for printing debug information.

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM. Just one minor nit on the option help string below. Thanks Comment at: clang/include/clang/Driver/CC1Options.td:329 +def fno_debug_pass_manager : Flag<["-"],

[PATCH] D34728: [ThinkLTO] Invoke build(Thin)?LTOPreLinkDefaultPipeline.

tejohnson added a comment. In https://reviews.llvm.org/D34728#794508, @timshen wrote: > Added -fexperimental-new-pass-manager=off/on/debug for printing debug > information. > > Added a Clang test. > > Do tell if you want me to split this patch. I didn't, becuase then I don't > have to write a

[PATCH] D34896: Enable the new PM + SamlePGO + ThinLTO testing.

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D34896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34721: [PM] Add support for sample PGO in the new pass manager (clang-side)

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM for when the dependent patches are in. https://reviews.llvm.org/D34721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34728: [ThinkLTO] Invoke build(Thin)?LTOPreLinkDefaultPipeline.

tejohnson added a comment. In https://reviews.llvm.org/D34728#793131, @timshen wrote: > A question I have is that I don't know how to test this. Ideally we want > -debug-pass-manager from opt, but that flag is not part of the LLVM libraries. How about add a clang test that builds with "-mllvm

[PATCH] D33134: Remove ignore-empty-index-file option

tejohnson created this revision. Herald added subscribers: eraman, mehdi_amini. Clang changes to remove this option and replace with a parameter always set in the context of a ThinLTO distributed backend. Depends on https://reviews.llvm.org/D33133. https://reviews.llvm.org/D33134 Files:

[PATCH] D33134: Remove ignore-empty-index-file option

This revision was automatically updated to reflect the committed changes. Closed by commit rL302940: Remove ignore-empty-index-file option (authored by tejohnson). Changed prior to commit: https://reviews.llvm.org/D33134?vs=98779=98821#toc Repository: rL LLVM

[PATCH] D34055: Be more strict when checking the -flto option value

tejohnson added a comment. In https://reviews.llvm.org/D34055#777005, @yamaguchi wrote: > I think we don't need additional testcase, because this is non-functional > change. Driver.cpp will emit error if value was not "thin" nor "full". This > testcase is at

[PATCH] D34546: docs: Add documentation for the ThinLTO cache pruning policy string.

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D34546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34055: Be more strict when checking the -flto option value

tejohnson added a comment. Make sure you add cfe-commits to the mailing list for clang changes (and llvm-commits for llvm changes, I accidentally added that one first and then fixed it), since all patches should go to the full mailing list. This fix looks correct to me. Please add a test case

[PATCH] D34055: Be more strict when checking the -flto option value

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM, thanks for the fix! https://reviews.llvm.org/D34055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34728: [ThinkLTO] Invoke build(Thin)?LTOPreLinkDefaultPipeline.

tejohnson added inline comments. Comment at: llvm/test/Other/new-pm-thinlto-defaults.ll:157 ; CHECK-PRELINK-O-NEXT: Running pass: GlobalOptPass -; CHECK-PRELINK-O-NEXT: Running pass: NameAnonGlobalPass ; CHECK-POSTLINK-O-NEXT: Running pass: PassManager<{{.*}}Module{{.*}}>

[PATCH] D37995: [Docs] Document cache pruning support for gold

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM with one fix noted. Comment at: docs/ThinLTO.rst:144 To help keep the size of the cache under control, ThinLTO supports cache pruning. Cache pruning is

[PATCH] D38517: Enabling new pass manager in LTO (and thinLTO) link step via -fexperimental-new-pass-manager option

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM thanks! Repository: rL LLVM https://reviews.llvm.org/D38517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38517: Enabling new pass manager in LTO (and thinLTO) link step via -fexperimental-new-pass-manager option

tejohnson added a comment. Thanks for adding this. Please add a test like the one for -ffunction-sections (tools/clang/test/Driver/gold-lto-sections.c). Repository: rL LLVM https://reviews.llvm.org/D38517 ___ cfe-commits mailing list

[PATCH] D40748: [ThinLTO] Enable importing of aliases as copy of aliasee

tejohnson created this revision. Herald added a subscriber: inglorion. Clang side changes to go with LLVM change to import aliases as a copy of their aliasee. Simply refactor out some handling that is moved to LLVM for use elsewhere. Depends on https://reviews.llvm.org/D40747. Repository: rC

[PATCH] D39923: [ThinLTO] Handle -fdebug-pass-manager for backend invocations via clang

tejohnson created this revision. Herald added subscribers: eraman, inglorion, mehdi_amini. The LTO Config field wasn't being set when invoking a ThinLTO backend via clang (i.e. for distributed builds). https://reviews.llvm.org/D39923 Files: lib/CodeGen/BackendUtil.cpp

[PATCH] D39923: [ThinLTO] Handle -fdebug-pass-manager for backend invocations via clang

This revision was automatically updated to reflect the committed changes. Closed by commit rL317951: [ThinLTO] Handle -fdebug-pass-manager for backend invocations via clang (authored by tejohnson). Repository: rL LLVM https://reviews.llvm.org/D39923 Files:

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

tejohnson marked an inline comment as done. tejohnson added a comment. Fixed the output file as suggested. Also note that this test will fail and can't go in until after https://reviews.llvm.org/D46387 Repository: rC Clang https://reviews.llvm.org/D46464

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

tejohnson updated this revision to Diff 145321. tejohnson added a comment. Address comments Repository: rC Clang https://reviews.llvm.org/D46464 Files: lib/CodeGen/BackendUtil.cpp lib/Frontend/CompilerInvocation.cpp test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

tejohnson updated this revision to Diff 145348. tejohnson added a comment. Update test for change to pass -1 as the Task ID for distributed backends, and to reflect companion llvm change (https://reviews.llvm.org/D46488). Repository: rC Clang https://reviews.llvm.org/D46464 Files:

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

tejohnson added inline comments. Comment at: test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll:14 +; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -fsave-optimization-record -fdiagnostics-show-hotness -o %t2.o -c +; RUN: cat %t2.opt.yaml.thin.0.yaml |

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

tejohnson created this revision. tejohnson added a reviewer: pcc. Herald added subscribers: eraman, inglorion, mehdi_amini. Passes down the necessary code ge options to the LTO Config to enable -fdiagnostics-show-hotness and -fsave-optimization-record in the ThinLTO backend for a distributed

[PATCH] D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests

tejohnson created this revision. tejohnson added a reviewer: pcc. Herald added subscribers: eraman, inglorion, mehdi_amini. Adds testing of combined index summary entries in disassembly format to CFI tests that were already testing the bitcode format. Depends on https://reviews.llvm.org/D46699.

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

This revision was automatically updated to reflect the committed changes. Closed by commit rL331592: [ThinLTO] Support opt remarks options with distributed ThinLTO backends (authored by tejohnson, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests

tejohnson updated this revision to Diff 146836. tejohnson added a comment. Update tests for changes to https://reviews.llvm.org/D46699 Repository: rC Clang https://reviews.llvm.org/D46700 Files: test/CodeGen/thinlto-distributed-cfi-devirt.ll test/CodeGen/thinlto-distributed-cfi.ll

[PATCH] D47906: [ThinLTO] Add testing of summary index parsing to a couple CFI tests

tejohnson created this revision. tejohnson added reviewers: pcc, dexonsmith, mehdi_amini. Herald added subscribers: steven_wu, eraman, inglorion. Changes to some clang side tests to go with the summary parsing patch. Depends on https://reviews.llvm.org/D47905. Repository: rC Clang

[PATCH] D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests

tejohnson updated this revision to Diff 148740. tejohnson added a comment. Update tests so they won't get bot failures (don't try to match path, module hash). Ping - this can go in now that https://reviews.llvm.org/D46699 is in. Repository: rC Clang https://reviews.llvm.org/D46700 Files:

[PATCH] D47597: IRGen: Write .dwo files when -split-dwarf-file is used together with -fthinlto-index.

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM thanks! https://reviews.llvm.org/D47597 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests

tejohnson added a comment. Herald added a subscriber: steven_wu. ping Repository: rC Clang https://reviews.llvm.org/D46700 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46700: [ThinLTO] Add testing of new summary index format to a couple CFI tests

This revision was automatically updated to reflect the committed changes. Closed by commit rL333966: [ThinLTO] Add testing of new summary index format to a couple CFI tests (authored by tejohnson, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D47906: [ThinLTO] Add testing of summary index parsing to a couple CFI tests

This revision was automatically updated to reflect the committed changes. Closed by commit rC335618: [ThinLTO] Add testing of summary index parsing to a couple CFI tests (authored by tejohnson, committed by ). Changed prior to commit: https://reviews.llvm.org/D47906?vs=150402=152899#toc

[PATCH] D47906: [ThinLTO] Add testing of summary index parsing to a couple CFI tests

tejohnson added a comment. ping (assuming this is ok, but since I sent for review...) Repository: rC Clang https://reviews.llvm.org/D47906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34156: [LTO] Enable module summary emission by default for regular LTO

tejohnson accepted this revision. tejohnson added a comment. LGTM with following suggestions. Comment at: clang/lib/CodeGen/BackendUtil.cpp:776 + bool EmitLTOSummary = + (CodeGenOpts.PrepareForLTO && !CodeGenOpts.PrepareForThinLTO && +

[PATCH] D42680: [ThinLTO] Ignore -fthinlto-index= for non-ThinLTO files

tejohnson added a comment. In https://reviews.llvm.org/D42680#991938, @pcc wrote: > This doesn't seem right to me. In a mixed full/thin LTO link the full LTO > module would be compiled during the indexing phase. We don't want to compile > it again in the backend as it could lead at best to

[PATCH] D42680: [ThinLTO] Ignore -fthinlto-index= for non-ThinLTO files

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM (minor comment fix in test needed) Comment at: clang/test/CodeGen/thinlto_backend.ll:23 +; Ensure we don't fail with index and non-ThinLTO object file, and run

[PATCH] D42680: [ThinLTO] Ignore object files with no ThinLTO modules if -fthinlto-index= is set

tejohnson added a comment. This looks ok to me, assuming it produces the empty output file as I expect it should. Please wait for pcc to take a look as well. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:959 +if (!Bm) + return {}; Expected

[PATCH] D42995: [ThinLTO] Ignore object files with empty ThinLTO index

tejohnson added a comment. > Empty ThinLTOIndexFile signals that we don't need this module during > linking. Not the only case actually. We now also pass an empty index file when we want to compile the bitcode down to object without applying any LTO optimization (there are a few cases where

[PATCH] D42995: [ThinLTO] Ignore object files with empty ThinLTO index

tejohnson added a comment. In https://reviews.llvm.org/D42995#1000155, @vitalybuka wrote: > In https://reviews.llvm.org/D42995#125, @tejohnson wrote: > > > > Empty ThinLTOIndexFile signals that we don't need this module during > > > linking. > > > > Not the only case actually. We now also

[PATCH] D42995: [ThinLTO] Allow indexing to request backend to ignore the module

tejohnson added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1176 + // Output of this module is not needed, but build system may not know + // that. So we need to generated empty valid object file. + EmptyModule = llvm::make_unique("empty",

[PATCH] D42995: [ThinLTO] Allow indexing to request backend to ignore the module

tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D42995 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50882: [ThinLTO] Correct documentation on default number of threads

tejohnson created this revision. tejohnson added a reviewer: pcc. Herald added subscribers: dexonsmith, steven_wu, eraman, inglorion, mehdi_amini. The number of threads used for ThinLTO backend parallelism was dropped to the number of cores in r284618 to avoid oversubscribing physical cores due

  1   2   3   4   5   6   7   >