[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
This revision was automatically updated to reflect the committed changes. Closed by commit rL359025: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM (authored by tejohnson, committed by ). Changed prior to commit: https://reviews.llvm.org/D61022?vs=196263&id=196296#toc Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61022/new/ https://reviews.llvm.org/D61022 Files: cfe/trunk/lib/CodeGen/BackendUtil.cpp cfe/trunk/test/CodeGen/thinlto-debug-pm.c cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll llvm/trunk/lib/Passes/PassBuilder.cpp llvm/trunk/test/tools/gold/X86/opt-level.ll Index: cfe/trunk/test/CodeGen/thinlto-debug-pm.c === --- cfe/trunk/test/CodeGen/thinlto-debug-pm.c +++ cfe/trunk/test/CodeGen/thinlto-debug-pm.c @@ -1,10 +1,17 @@ -// Test to ensure -fdebug-pass-manager works when invoking the -// ThinLTO backend path with the new PM. +// Test to ensure the opt level is passed down to the ThinLTO backend. // REQUIRES: x86-registered-target // RUN: %clang_cc1 -o %t.o -flto=thin -fexperimental-new-pass-manager -triple x86_64-unknown-linux-gnu -emit-llvm-bc %s // RUN: llvm-lto -thinlto -o %t %t.o -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O2 -o %t2.o -x ir %t.o -fthinlto-index=%t.thinlto.bc -fdebug-pass-manager -fexperimental-new-pass-manager 2>&1 | FileCheck %s -// CHECK: Running pass: + +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O2 -o %t2.o -x ir %t.o -fthinlto-index=%t.thinlto.bc -fdebug-pass-manager -fexperimental-new-pass-manager 2>&1 | FileCheck %s --check-prefix=O2-NEWPM +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O0 -o %t2.o -x ir %t.o -fthinlto-index=%t.thinlto.bc -fdebug-pass-manager -fexperimental-new-pass-manager 2>&1 | FileCheck %s --check-prefix=O0-NEWPM +// O2-NEWPM: Running pass: LoopVectorizePass +// O0-NEWPM-NOT: Running pass: LoopVectorizePass + +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O2 -o %t2.o -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm -debug-pass=Structure 2>&1 | FileCheck %s --check-prefix=O2-OLDPM +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O0 -o %t2.o -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm -debug-pass=Structure 2>&1 | FileCheck %s --check-prefix=O0-OLDPM +// O2-OLDPM: Loop Vectorization +// O0-OLDPM-NOT: Loop Vectorization void foo() { } Index: cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll === --- cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll +++ cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll @@ -39,12 +39,12 @@ ; CHECK-DIS: ^2 = typeid: (name: "_ZTS1A", summary: (typeTestRes: (kind: allOnes, sizeM1BitWidth: 7), wpdResolutions: ((offset: 0, wpdRes: (kind: branchFunnel)), (offset: 8, wpdRes: (kind: singleImpl, singleImplName: "_ZN1A1nEi") ; guid = 7004155349499253778 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \ -; RUN: -emit-obj -fthinlto-index=%t.o.thinlto.bc \ +; RUN: -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 \ ; RUN: -emit-llvm -o - -x ir %t.o | FileCheck %s --check-prefixes=CHECK-IR ; Check that backend does not fail generating native code. ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \ -; RUN: -emit-obj -fthinlto-index=%t.o.thinlto.bc \ +; RUN: -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 \ ; RUN: -o %t.native.o -x ir %t.o target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp === --- cfe/trunk/lib/CodeGen/BackendUtil.cpp +++ cfe/trunk/lib/CodeGen/BackendUtil.cpp @@ -1325,6 +1325,7 @@ Conf.MAttrs = TOpts.Features; Conf.RelocModel = CGOpts.RelocationModel; Conf.CGOptLevel = getCGOptLevel(CGOpts); + Conf.OptLevel = CGOpts.OptimizationLevel; initTargetOptions(Conf.Options, CGOpts, TOpts, LOpts, HeaderOpts); Conf.SampleProfile = std::move(SampleProfile); Index: llvm/trunk/lib/Passes/PassBuilder.cpp === --- llvm/trunk/lib/Passes/PassBuilder.cpp +++ llvm/trunk/lib/Passes/PassBuilder.cpp @@ -1046,10 +1046,16 @@ // // Also, WPD has access to more precise information than ICP and can // devirtualize more effectively, so it should operate on the IR first. +// +// The WPD and LowerTypeTest passes need to run at -O0 to lower type +// metadata and intrinsics. MPM.addPass(WholeProgramDevirtPass(nullptr, ImportSummary)); MPM.addPass(LowerTypeTestsPass(nullptr, ImportSummary)); } + if (Level == O0) +return MPM; + // Force any function attributes we want the rest of the pipeline to observe. MPM.addPass(ForceFunctionAttrsPass()); @@ -1075,9 +1081,16 @@ ModulePassManager PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level, bool DebugL
[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
mehdi_amini added inline comments. Comment at: llvm/test/tools/gold/X86/opt-level.ll:53 + ; CHECK-O1-OLDPM: select + ; The new PM does not do as many optimizations at O1 + ; CHECK-O1-NEWPM: phi tejohnson wrote: > chandlerc wrote: > > tejohnson wrote: > > > tejohnson wrote: > > > > mehdi_amini wrote: > > > > > This is intended? I'm surprised the two PMs don't have the same list > > > > > of passes for a given opt level? > > > > I'm really not sure. I did compare the post-link LTO pipelines of both > > > > PMs at O0/O1/O2 and confirmed that the old PM is doing many more passes > > > > than the new PM at O1. Probably a question for @chandlerc ? In any > > > > case, I didn't want to address it here since it is orthogonal. > > > Some more info: > > > > > > This is the regular LTO post-link pipeline, ThinLTO post-link pipeline > > > uses essentially the same as a normal opt pipeline so would be consistent > > > at -O1. > > > > > > The optimization missing from the new PM regular LTO post link pipeline > > > that affects this test is SimplifyCFG. This and a few other optimizations > > > are added in the old PM at O1 via > > > PassManagerBuilder::addLateLTOOptimizationPasses. Note that > > > PassManagerBuilder::addLTOOptimizationPasses does exit early at -O1 > > > (similar to where we exit early in the new PM for the regular LTO post > > > link compilation). But the "late" LTO optimization passes are added > > > unconditionally above -O0. Is this correct/desired? There isn't an > > > equivalent in the new PM. > > I don't think it was intentional, but I'm not sure I would directly > > replicate it if it requires adding an entirely new customization point. Is > > their some simpler way of getting equivalent results at O1? > Yeah we can presumably just add those optimizations to the -O1 early exit > path in PassBuilder::buildLTODefaultPipeline. I can send a patch (but would > like to get this one in as it is a bugfix and orthogonal). (I fully agree it is orthogonal, I just spotted this as a separate bug indeed, thanks for fixing!) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61022/new/ https://reviews.llvm.org/D61022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
xur accepted this revision. xur added a comment. This revision is now accepted and ready to land. LGTM. We need to Initialize the OptLevel no matter what. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61022/new/ https://reviews.llvm.org/D61022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
tejohnson marked an inline comment as done. tejohnson added inline comments. Comment at: llvm/test/tools/gold/X86/opt-level.ll:53 + ; CHECK-O1-OLDPM: select + ; The new PM does not do as many optimizations at O1 + ; CHECK-O1-NEWPM: phi chandlerc wrote: > tejohnson wrote: > > tejohnson wrote: > > > mehdi_amini wrote: > > > > This is intended? I'm surprised the two PMs don't have the same list of > > > > passes for a given opt level? > > > I'm really not sure. I did compare the post-link LTO pipelines of both > > > PMs at O0/O1/O2 and confirmed that the old PM is doing many more passes > > > than the new PM at O1. Probably a question for @chandlerc ? In any case, > > > I didn't want to address it here since it is orthogonal. > > Some more info: > > > > This is the regular LTO post-link pipeline, ThinLTO post-link pipeline uses > > essentially the same as a normal opt pipeline so would be consistent at -O1. > > > > The optimization missing from the new PM regular LTO post link pipeline > > that affects this test is SimplifyCFG. This and a few other optimizations > > are added in the old PM at O1 via > > PassManagerBuilder::addLateLTOOptimizationPasses. Note that > > PassManagerBuilder::addLTOOptimizationPasses does exit early at -O1 > > (similar to where we exit early in the new PM for the regular LTO post link > > compilation). But the "late" LTO optimization passes are added > > unconditionally above -O0. Is this correct/desired? There isn't an > > equivalent in the new PM. > I don't think it was intentional, but I'm not sure I would directly replicate > it if it requires adding an entirely new customization point. Is their some > simpler way of getting equivalent results at O1? Yeah we can presumably just add those optimizations to the -O1 early exit path in PassBuilder::buildLTODefaultPipeline. I can send a patch (but would like to get this one in as it is a bugfix and orthogonal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61022/new/ https://reviews.llvm.org/D61022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
chandlerc added inline comments. Comment at: llvm/test/tools/gold/X86/opt-level.ll:53 + ; CHECK-O1-OLDPM: select + ; The new PM does not do as many optimizations at O1 + ; CHECK-O1-NEWPM: phi tejohnson wrote: > tejohnson wrote: > > mehdi_amini wrote: > > > This is intended? I'm surprised the two PMs don't have the same list of > > > passes for a given opt level? > > I'm really not sure. I did compare the post-link LTO pipelines of both PMs > > at O0/O1/O2 and confirmed that the old PM is doing many more passes than > > the new PM at O1. Probably a question for @chandlerc ? In any case, I > > didn't want to address it here since it is orthogonal. > Some more info: > > This is the regular LTO post-link pipeline, ThinLTO post-link pipeline uses > essentially the same as a normal opt pipeline so would be consistent at -O1. > > The optimization missing from the new PM regular LTO post link pipeline that > affects this test is SimplifyCFG. This and a few other optimizations are > added in the old PM at O1 via > PassManagerBuilder::addLateLTOOptimizationPasses. Note that > PassManagerBuilder::addLTOOptimizationPasses does exit early at -O1 (similar > to where we exit early in the new PM for the regular LTO post link > compilation). But the "late" LTO optimization passes are added > unconditionally above -O0. Is this correct/desired? There isn't an equivalent > in the new PM. I don't think it was intentional, but I'm not sure I would directly replicate it if it requires adding an entirely new customization point. Is their some simpler way of getting equivalent results at O1? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61022/new/ https://reviews.llvm.org/D61022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
tejohnson marked an inline comment as done. tejohnson added inline comments. Comment at: llvm/test/tools/gold/X86/opt-level.ll:53 + ; CHECK-O1-OLDPM: select + ; The new PM does not do as many optimizations at O1 + ; CHECK-O1-NEWPM: phi tejohnson wrote: > mehdi_amini wrote: > > This is intended? I'm surprised the two PMs don't have the same list of > > passes for a given opt level? > I'm really not sure. I did compare the post-link LTO pipelines of both PMs at > O0/O1/O2 and confirmed that the old PM is doing many more passes than the new > PM at O1. Probably a question for @chandlerc ? In any case, I didn't want to > address it here since it is orthogonal. Some more info: This is the regular LTO post-link pipeline, ThinLTO post-link pipeline uses essentially the same as a normal opt pipeline so would be consistent at -O1. The optimization missing from the new PM regular LTO post link pipeline that affects this test is SimplifyCFG. This and a few other optimizations are added in the old PM at O1 via PassManagerBuilder::addLateLTOOptimizationPasses. Note that PassManagerBuilder::addLTOOptimizationPasses does exit early at -O1 (similar to where we exit early in the new PM for the regular LTO post link compilation). But the "late" LTO optimization passes are added unconditionally above -O0. Is this correct/desired? There isn't an equivalent in the new PM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61022/new/ https://reviews.llvm.org/D61022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
tejohnson marked an inline comment as done. tejohnson added inline comments. Comment at: llvm/test/tools/gold/X86/opt-level.ll:53 + ; CHECK-O1-OLDPM: select + ; The new PM does not do as many optimizations at O1 + ; CHECK-O1-NEWPM: phi mehdi_amini wrote: > This is intended? I'm surprised the two PMs don't have the same list of > passes for a given opt level? I'm really not sure. I did compare the post-link LTO pipelines of both PMs at O0/O1/O2 and confirmed that the old PM is doing many more passes than the new PM at O1. Probably a question for @chandlerc ? In any case, I didn't want to address it here since it is orthogonal. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61022/new/ https://reviews.llvm.org/D61022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
mehdi_amini added inline comments. Comment at: llvm/test/tools/gold/X86/opt-level.ll:53 + ; CHECK-O1-OLDPM: select + ; The new PM does not do as many optimizations at O1 + ; CHECK-O1-NEWPM: phi This is intended? I'm surprised the two PMs don't have the same list of passes for a given opt level? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61022/new/ https://reviews.llvm.org/D61022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM
tejohnson created this revision. tejohnson added a reviewer: xur. Herald added subscribers: dexonsmith, steven_wu, hiraditya, eraman, inglorion, mehdi_amini. Herald added projects: clang, LLVM. The opt level was not being passed down to the ThinLTO backend when invoked via clang (for distributed ThinLTO). This exposed an issue where the new PM was asserting if the Thin or regular LTO backend pipelines were invoked with -O0 (not a new issue, could be provoked by invoking in-process *LTO backends via linker using new PM and -O0). Fix this similar to the old PM where -O0 only does the necessary lowering of type metadata (WPD and LowerTypeTest passes) and then quits, rather than asserting. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D61022 Files: clang/lib/CodeGen/BackendUtil.cpp clang/test/CodeGen/thinlto-debug-pm.c clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll llvm/lib/Passes/PassBuilder.cpp llvm/test/tools/gold/X86/opt-level.ll Index: llvm/test/tools/gold/X86/opt-level.ll === --- llvm/test/tools/gold/X86/opt-level.ll +++ llvm/test/tools/gold/X86/opt-level.ll @@ -6,12 +6,25 @@ ; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext -plugin-opt=save-temps \ ; RUN:-m elf_x86_64 \ ; RUN:-plugin-opt=O1 -r -o %t.o %t.bc -; RUN: llvm-dis < %t.o.0.4.opt.bc -o - | FileCheck --check-prefix=CHECK-O1 %s +; RUN: llvm-dis < %t.o.0.4.opt.bc -o - | FileCheck --check-prefix=CHECK-O1 --check-prefix=CHECK-O1-OLDPM %s ; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext -plugin-opt=save-temps \ ; RUN:-m elf_x86_64 \ ; RUN:-plugin-opt=O2 -r -o %t.o %t.bc ; RUN: llvm-dis < %t.o.0.4.opt.bc -o - | FileCheck --check-prefix=CHECK-O2 %s +; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext -plugin-opt=save-temps \ +; RUN:-m elf_x86_64 --plugin-opt=new-pass-manager \ +; RUN:-plugin-opt=O0 -r -o %t.o %t.bc +; RUN: llvm-dis < %t.o.0.4.opt.bc -o - | FileCheck --check-prefix=CHECK-O0 %s +; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext -plugin-opt=save-temps \ +; RUN:-m elf_x86_64 --plugin-opt=new-pass-manager \ +; RUN:-plugin-opt=O1 -r -o %t.o %t.bc +; RUN: llvm-dis < %t.o.0.4.opt.bc -o - | FileCheck --check-prefix=CHECK-O1 --check-prefix=CHECK-O1-NEWPM %s +; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext -plugin-opt=save-temps \ +; RUN:-m elf_x86_64 --plugin-opt=new-pass-manager \ +; RUN:-plugin-opt=O2 -r -o %t.o %t.bc +; RUN: llvm-dis < %t.o.0.4.opt.bc -o - | FileCheck --check-prefix=CHECK-O2 %s + ; CHECK-O0: define internal void @foo( ; CHECK-O1: define internal void @foo( ; CHECK-O2-NOT: define internal void @foo( @@ -36,7 +49,9 @@ end: ; CHECK-O0: phi - ; CHECK-O1: select + ; CHECK-O1-OLDPM: select + ; The new PM does not do as many optimizations at O1 + ; CHECK-O1-NEWPM: phi %r = phi i32 [ 1, %t ], [ 2, %f ] ret i32 %r } Index: llvm/lib/Passes/PassBuilder.cpp === --- llvm/lib/Passes/PassBuilder.cpp +++ llvm/lib/Passes/PassBuilder.cpp @@ -1046,10 +1046,16 @@ // // Also, WPD has access to more precise information than ICP and can // devirtualize more effectively, so it should operate on the IR first. +// +// The WPD and LowerTypeTest passes need to run at -O0 to lower type +// metadata and intrinsics. MPM.addPass(WholeProgramDevirtPass(nullptr, ImportSummary)); MPM.addPass(LowerTypeTestsPass(nullptr, ImportSummary)); } + if (Level == O0) +return MPM; + // Force any function attributes we want the rest of the pipeline to observe. MPM.addPass(ForceFunctionAttrsPass()); @@ -1075,9 +1081,16 @@ ModulePassManager PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level, bool DebugLogging, ModuleSummaryIndex *ExportSummary) { - assert(Level != O0 && "Must request optimizations for the default pipeline!"); ModulePassManager MPM(DebugLogging); + if (Level == O0) { +// The WPD and LowerTypeTest passes need to run at -O0 to lower type +// metadata and intrinsics. +MPM.addPass(WholeProgramDevirtPass(ExportSummary, nullptr)); +MPM.addPass(LowerTypeTestsPass(ExportSummary, nullptr)); +return MPM; + } + if (PGOOpt && PGOOpt->Action == PGOOptions::SampleUse) { // Load sample profile before running the LTO optimization pipeline. MPM.addPass(SampleProfileLoaderPass(PGOOpt->ProfileFile, Index: clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll === --- clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll +++ clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll @@ -39,12 +39,12 @@ ; CHECK-DIS: ^2 = typeid: (name: "_ZTS1A", summary: (typeTestRes: (kind: allOnes, sizeM1BitWidth: 7), wpdResolutions: ((offset: 0, wpdRes: (kind: branchFunnel)), (offset: 8, wpdRes: (kind: singleImpl, singleImplName: "_ZN1A1nEi")