From 2b3598bd043f5ebcefb9a1c8576ce45e2ee65595 Mon Sep 17 00:00:00 2001 From: Paul Kirth Date: Wed, 30 Oct 2024 16:56:30 -0700 Subject: [PATCH] [llvm] Allow always dropping all llvm.type.test sequences Currently, the `DropTypeTests` parameter only fully works with phi nodes and llvm.assume instructions. However, we'd like CFI to work in conjunction with FatLTO, in so far as the bitcode section should be able to contain the CFI instrumentation, while any incompatible bits are dropped when compiling the object code. To do that, we need to drop the llvm.type.test instructions everywhere, and not just their uses in phi nodes. This patch updates the LowerTypeTest pass so that uses are removed, and replaced with `true` in all cases, and not just in phi nodes. Addressing this will allow us to fix #112053 by modifying the FatLTO pipeline. Reviewers: pcc, nikic Reviewed By: pcc Pull Request: https://github.com/llvm/llvm-project/pull/112787 --- clang/lib/CodeGen/BackendUtil.cpp | 7 ++-- .../llvm/Transforms/IPO/LowerTypeTests.h | 13 +++++- llvm/lib/Passes/PassBuilderPipelines.cpp | 15 ++++--- llvm/lib/Transforms/IPO/LowerTypeTests.cpp | 41 ++++++++++++------- .../LowerTypeTests/drop_type_test.ll | 22 ++++++++++ .../LowerTypeTests/drop_type_test_phi.ll | 2 +- 6 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 llvm/test/Transforms/LowerTypeTests/drop_type_test.ll diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index f018130807519..ae33554a66b6b 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -1013,9 +1013,10 @@ void EmitAssemblyHelper::RunOptimizationPipeline( if (IsThinLTOPostLink) PB.registerPipelineStartEPCallback( [](ModulePassManager &MPM, OptimizationLevel Level) { - MPM.addPass(LowerTypeTestsPass(/*ExportSummary=*/nullptr, - /*ImportSummary=*/nullptr, - /*DropTypeTests=*/true)); + MPM.addPass(LowerTypeTestsPass( + /*ExportSummary=*/nullptr, + /*ImportSummary=*/nullptr, + /*DropTypeTests=*/lowertypetests::DropTestKind::Assume)); }); // Register callbacks to schedule sanitizer passes at the appropriate part diff --git a/llvm/include/llvm/Transforms/IPO/LowerTypeTests.h b/llvm/include/llvm/Transforms/IPO/LowerTypeTests.h index eb682c437b94b..02adcd8bfd45d 100644 --- a/llvm/include/llvm/Transforms/IPO/LowerTypeTests.h +++ b/llvm/include/llvm/Transforms/IPO/LowerTypeTests.h @@ -195,6 +195,13 @@ struct ByteArrayBuilder { bool isJumpTableCanonical(Function *F); +/// Specifies how to drop type tests. +enum class DropTestKind { + None, /// Do not drop type tests (default). + Assume, /// Drop only llvm.assumes using type test value. + All, /// Drop the type test and all uses. +}; + } // end namespace lowertypetests class LowerTypeTestsPass : public PassInfoMixin { @@ -202,13 +209,15 @@ class LowerTypeTestsPass : public PassInfoMixin { ModuleSummaryIndex *ExportSummary = nullptr; const ModuleSummaryIndex *ImportSummary = nullptr; - bool DropTypeTests = true; + lowertypetests::DropTestKind DropTypeTests = + lowertypetests::DropTestKind::None; public: LowerTypeTestsPass() : UseCommandLine(true) {} LowerTypeTestsPass(ModuleSummaryIndex *ExportSummary, const ModuleSummaryIndex *ImportSummary, - bool DropTypeTests = false) + lowertypetests::DropTestKind DropTypeTests = + lowertypetests::DropTestKind::None) : ExportSummary(ExportSummary), ImportSummary(ImportSummary), DropTypeTests(DropTypeTests) {} PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM); diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp index 488554c84c1c4..c391853c8d0c2 100644 --- a/llvm/lib/Passes/PassBuilderPipelines.cpp +++ b/llvm/lib/Passes/PassBuilderPipelines.cpp @@ -1135,7 +1135,8 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, // post link pipeline after ICP. This is to enable usage of the type // tests in ICP sequences. if (Phase == ThinOrFullLTOPhase::ThinLTOPostLink) - MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, true)); + MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, + lowertypetests::DropTestKind::Assume)); invokePipelineEarlySimplificationEPCallbacks(MPM, Level); @@ -1750,7 +1751,8 @@ ModulePassManager PassBuilder::buildThinLTODefaultPipeline( if (Level == OptimizationLevel::O0) { // Run a second time to clean up any type tests left behind by WPD for use // in ICP. - MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, true)); + MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, + lowertypetests::DropTestKind::Assume)); // Drop available_externally and unreferenced globals. This is necessary // with ThinLTO in order to avoid leaving undefined references to dead // globals in the object file. @@ -1801,7 +1803,8 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level, MPM.addPass(LowerTypeTestsPass(ExportSummary, nullptr)); // Run a second time to clean up any type tests left behind by WPD for use // in ICP. - MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, true)); + MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, + lowertypetests::DropTestKind::Assume)); invokeFullLinkTimeOptimizationLastEPCallbacks(MPM, Level); @@ -1879,7 +1882,8 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level, // Run a second time to clean up any type tests left behind by WPD for use // in ICP (which is performed earlier than this in the regular LTO // pipeline). - MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, true)); + MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, + lowertypetests::DropTestKind::Assume)); invokeFullLinkTimeOptimizationLastEPCallbacks(MPM, Level); @@ -2060,7 +2064,8 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level, MPM.addPass(LowerTypeTestsPass(ExportSummary, nullptr)); // Run a second time to clean up any type tests left behind by WPD for use // in ICP (which is performed earlier than this in the regular LTO pipeline). - MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, true)); + MPM.addPass(LowerTypeTestsPass(nullptr, nullptr, + lowertypetests::DropTestKind::Assume)); // Enable splitting late in the FullLTO post-link pipeline. if (EnableHotColdSplit) diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp index 3fcfc6a876776..9369b91d9c7f1 100644 --- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp +++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp @@ -118,10 +118,16 @@ static cl::opt ClWriteSummary( cl::desc("Write summary to given YAML file after running pass"), cl::Hidden); -static cl::opt +static cl::opt ClDropTypeTests("lowertypetests-drop-type-tests", - cl::desc("Simply drop type test assume sequences"), - cl::Hidden, cl::init(false)); + cl::desc("Simply drop type test sequences"), + cl::values(clEnumValN(DropTestKind::None, "none", + "Do not drop any type tests"), + clEnumValN(DropTestKind::Assume, "assume", + "Drop type test assume sequences"), + clEnumValN(DropTestKind::All, "all", + "Drop all type test sequences")), + cl::Hidden, cl::init(DropTestKind::None)); bool BitSetInfo::containsGlobalOffset(uint64_t Offset) const { if (Offset < ByteOffset) @@ -399,7 +405,7 @@ class LowerTypeTestsModule { const ModuleSummaryIndex *ImportSummary; // Set when the client has invoked this to simply drop all type test assume // sequences. - bool DropTypeTests; + DropTestKind DropTypeTests; Triple::ArchType Arch; Triple::OSType OS; @@ -542,7 +548,7 @@ class LowerTypeTestsModule { LowerTypeTestsModule(Module &M, ModuleAnalysisManager &AM, ModuleSummaryIndex *ExportSummary, const ModuleSummaryIndex *ImportSummary, - bool DropTypeTests); + DropTestKind DropTypeTests); bool lower(); @@ -1828,9 +1834,10 @@ void LowerTypeTestsModule::buildBitSetsFromDisjointSet( /// Lower all type tests in this module. LowerTypeTestsModule::LowerTypeTestsModule( Module &M, ModuleAnalysisManager &AM, ModuleSummaryIndex *ExportSummary, - const ModuleSummaryIndex *ImportSummary, bool DropTypeTests) + const ModuleSummaryIndex *ImportSummary, DropTestKind DropTypeTests) : M(M), ExportSummary(ExportSummary), ImportSummary(ImportSummary), - DropTypeTests(DropTypeTests || ClDropTypeTests) { + DropTypeTests(ClDropTypeTests > DropTypeTests ? ClDropTypeTests + : DropTypeTests) { assert(!(ExportSummary && ImportSummary)); Triple TargetTriple(M.getTargetTriple()); Arch = TargetTriple.getArch(); @@ -1882,7 +1889,7 @@ bool LowerTypeTestsModule::runForTesting(Module &M, ModuleAnalysisManager &AM) { M, AM, ClSummaryAction == PassSummaryAction::Export ? &Summary : nullptr, ClSummaryAction == PassSummaryAction::Import ? &Summary : nullptr, - /*DropTypeTests*/ false) + /*DropTypeTests=*/DropTestKind::None) .lower(); if (!ClWriteSummary.empty()) { @@ -1949,7 +1956,8 @@ void LowerTypeTestsModule::replaceDirectCalls(Value *Old, Value *New) { Old->replaceUsesWithIf(New, isDirectCall); } -static void dropTypeTests(Module &M, Function &TypeTestFunc) { +static void dropTypeTests(Module &M, Function &TypeTestFunc, + bool ShouldDropAll) { for (Use &U : llvm::make_early_inc_range(TypeTestFunc.uses())) { auto *CI = cast(U.getUser()); // Find and erase llvm.assume intrinsics for this llvm.type.test call. @@ -1959,9 +1967,13 @@ static void dropTypeTests(Module &M, Function &TypeTestFunc) { // If the assume was merged with another assume, we might have a use on a // phi (which will feed the assume). Simply replace the use on the phi // with "true" and leave the merged assume. + // + // If ShouldDropAll is set, then we we need to update any remaining uses, + // regardless of the instruction type. if (!CI->use_empty()) { - assert( - all_of(CI->users(), [](User *U) -> bool { return isa(U); })); + assert(ShouldDropAll || all_of(CI->users(), [](User *U) -> bool { + return isa(U); + })); CI->replaceAllUsesWith(ConstantInt::getTrue(M.getContext())); } CI->eraseFromParent(); @@ -1972,16 +1984,17 @@ bool LowerTypeTestsModule::lower() { Function *TypeTestFunc = Intrinsic::getDeclarationIfExists(&M, Intrinsic::type_test); - if (DropTypeTests) { + if (DropTypeTests != DropTestKind::None) { + bool ShouldDropAll = DropTypeTests == DropTestKind::All; if (TypeTestFunc) - dropTypeTests(M, *TypeTestFunc); + dropTypeTests(M, *TypeTestFunc, ShouldDropAll); // Normally we'd have already removed all @llvm.public.type.test calls, // except for in the case where we originally were performing ThinLTO but // decided not to in the backend. Function *PublicTypeTestFunc = Intrinsic::getDeclarationIfExists(&M, Intrinsic::public_type_test); if (PublicTypeTestFunc) - dropTypeTests(M, *PublicTypeTestFunc); + dropTypeTests(M, *PublicTypeTestFunc, ShouldDropAll); if (TypeTestFunc || PublicTypeTestFunc) { // We have deleted the type intrinsics, so we no longer have enough // information to reason about the liveness of virtual function pointers diff --git a/llvm/test/Transforms/LowerTypeTests/drop_type_test.ll b/llvm/test/Transforms/LowerTypeTests/drop_type_test.ll new file mode 100644 index 0000000000000..e1d0573924a4e --- /dev/null +++ b/llvm/test/Transforms/LowerTypeTests/drop_type_test.ll @@ -0,0 +1,22 @@ +; RUN: opt -S -passes=lowertypetests -lowertypetests-drop-type-tests=all < %s | FileCheck %s + +define void @func() { +entry: + %0 = tail call i1 @llvm.type.test(ptr null, metadata !"foo") + br i1 %0, label %exit, label %trap + +trap: + unreachable + +exit: + ret void + ; CHECK-LABEL: entry: + ; CHECK-NEXT: br i1 true, label %exit, label %trap + ; CHECK-LABEL: trap: + ; CHECK-NEXT: unreachable + ; CHECK-LABEL: exit: + ; CHECK-NEXT: ret void +} + +declare i1 @llvm.type.test(ptr, metadata) #0 +attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) } diff --git a/llvm/test/Transforms/LowerTypeTests/drop_type_test_phi.ll b/llvm/test/Transforms/LowerTypeTests/drop_type_test_phi.ll index 3cf4d447605da..820865826dc7a 100644 --- a/llvm/test/Transforms/LowerTypeTests/drop_type_test_phi.ll +++ b/llvm/test/Transforms/LowerTypeTests/drop_type_test_phi.ll @@ -1,5 +1,5 @@ ; Test to ensure dropping of type tests can handle a phi feeding the assume. -; RUN: opt -S -passes=lowertypetests -lowertypetests-drop-type-tests -mtriple=x86_64-unknown-linux-gnu %s | FileCheck %s +; RUN: opt -S -passes=lowertypetests -lowertypetests-drop-type-tests=assume -mtriple=x86_64-unknown-linux-gnu %s | FileCheck %s target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-grtev4-linux-gnu"