Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions llvm/include/llvm/Transforms/IPO/LowerTypeTests.h
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,11 @@ class LowerTypeTestsPass : public PassInfoMixin<LowerTypeTestsPass> {
PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
};

class SimplifyTypeTestsPass : public PassInfoMixin<SimplifyTypeTestsPass> {
public:
PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
};

} // end namespace llvm

#endif // LLVM_TRANSFORMS_IPO_LOWERTYPETESTS_H
3 changes: 3 additions & 0 deletions llvm/lib/Passes/PassBuilderPipelines.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1288,6 +1288,9 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
// and argument promotion.
MPM.addPass(DeadArgumentEliminationPass());

if (Phase == ThinOrFullLTOPhase::ThinLTOPostLink)
MPM.addPass(SimplifyTypeTestsPass());

if (Phase != ThinOrFullLTOPhase::ThinLTOPreLink)
MPM.addPass(CoroCleanupPass());

Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Passes/PassRegistry.def
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ MODULE_PASS("jmc-instrumenter", JMCInstrumenterPass())
MODULE_PASS("lower-emutls", LowerEmuTLSPass())
MODULE_PASS("lower-global-dtors", LowerGlobalDtorsPass())
MODULE_PASS("lower-ifunc", LowerIFuncPass())
MODULE_PASS("simplify-type-tests", SimplifyTypeTestsPass())
MODULE_PASS("lowertypetests", LowerTypeTestsPass())
MODULE_PASS("fatlto-cleanup", FatLtoCleanup())
MODULE_PASS("pgo-force-function-attrs", PGOForceFunctionAttrsPass(PGOOpt ? PGOOpt->ColdOptType : PGOOptions::ColdFuncOpt::Default))
Expand Down
93 changes: 93 additions & 0 deletions llvm/lib/Transforms/IPO/LowerTypeTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/EquivalenceClasses.h"
#include "llvm/ADT/PointerUnion.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/TinyPtrVector.h"
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/PostDominators.h"
#include "llvm/Analysis/TargetTransformInfo.h"
#include "llvm/Analysis/TypeMetadataUtils.h"
#include "llvm/Analysis/ValueTracking.h"
Expand Down Expand Up @@ -2472,3 +2475,93 @@ PreservedAnalyses LowerTypeTestsPass::run(Module &M,
return PreservedAnalyses::all();
return PreservedAnalyses::none();
}

PreservedAnalyses SimplifyTypeTestsPass::run(Module &M,
ModuleAnalysisManager &AM) {
bool Changed = false;
// Figure out whether inlining has exposed a constant address to a lowered
// type test, and remove the test if so and the address is known to pass the
// test. Unfortunately this pass ends up needing to reverse engineer what
// LowerTypeTests did; this is currently inherent to the design of ThinLTO
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a more extensive comment with what this is looking for and why? I don't look at lower type test output often so I don't recall offhand what e.g. it would have looked like without inlining vs with.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

// importing where LowerTypeTests needs to run at the start.
//
// We look for things like:
//
// sub (i64 ptrtoint (ptr @_Z2fpv to i64), i64 ptrtoint (ptr @__typeid__ZTSFvvE_global_addr to i64))
//
// which gets replaced with 0 if _Z2fpv (more specifically _Z2fpv.cfi, the
// function referred to by the jump table) is a member of the type _ZTSFvv, as
// well as things like
//
// icmp eq ptr @_Z2fpv, @__typeid__ZTSFvvE_global_addr
//
// which gets replaced with true if _Z2fpv is a member.
for (auto &GV : M.globals()) {
if (!GV.getName().starts_with("__typeid_") ||
!GV.getName().ends_with("_global_addr"))
continue;
// __typeid_foo_global_addr -> foo
auto *MD = MDString::get(M.getContext(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment on this conversion? Figured it out by adding up the chars myself but it would be good to make it explicit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just use strlen("__typeid_") and strlen("_global_addr")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added an example, I think that should make it clear enough.

GV.getName().substr(9, GV.getName().size() - 21));
auto MaySimplifyPtr = [&](Value *Ptr) {
if (auto *GV = dyn_cast<GlobalValue>(Ptr))
if (auto *CFIGV = M.getNamedValue((GV->getName() + ".cfi").str()))
Ptr = CFIGV;
return isKnownTypeIdMember(MD, M.getDataLayout(), Ptr, 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there cases where the GV will not have a ".cfi" extension? I notice the test has that extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if it's a function defined outside the LTO unit the jump table entry will be named foo.cfi_jt and the external function will be named foo. Since functions inside the LTO unit are the more common case I only handled them here.

};
auto MaySimplifyInt = [&](Value *Op) {
auto *PtrAsInt = dyn_cast<ConstantExpr>(Op);
if (!PtrAsInt || PtrAsInt->getOpcode() != Instruction::PtrToInt)
return false;
return MaySimplifyPtr(PtrAsInt->getOperand(0));
};
for (User *U : make_early_inc_range(GV.users())) {
if (auto *CI = dyn_cast<ICmpInst>(U)) {
if (CI->getPredicate() == CmpInst::ICMP_EQ &&
MaySimplifyPtr(CI->getOperand(0))) {
// This is an equality comparison (TypeTestResolution::Single case in
// lowerTypeTestCall). In this case we just replace the comparison
// with true.
CI->replaceAllUsesWith(ConstantInt::getTrue(M.getContext()));
CI->eraseFromParent();
Changed = true;
}
}
auto *CE = dyn_cast<ConstantExpr>(U);
if (!CE || CE->getOpcode() != Instruction::PtrToInt)
continue;
for (Use &U : make_early_inc_range(CE->uses())) {
auto *CE = dyn_cast<ConstantExpr>(U.getUser());
if (U.getOperandNo() == 1 && CE &&
CE->getOpcode() == Instruction::Sub &&
MaySimplifyInt(CE->getOperand(0))) {
// This is a computation of PtrOffset as generated by
// LowerTypeTestsModule::lowerTypeTestCall above. If
// isKnownTypeIdMember passes we just pretend it evaluated to 0. This
// should cause later passes to remove the range and alignment checks.
// The bitset checks won't be removed but those are uncommon.
CE->replaceAllUsesWith(ConstantInt::get(CE->getType(), 0));
Changed = true;
}
auto *CI = dyn_cast<ICmpInst>(U.getUser());
if (U.getOperandNo() == 1 && CI &&
CI->getPredicate() == CmpInst::ICMP_EQ &&
MaySimplifyInt(CI->getOperand(0))) {
// This is an equality comparison. Unlike in the case above it
// remained as an integer compare.
CI->replaceAllUsesWith(ConstantInt::getTrue(M.getContext()));
CI->eraseFromParent();
Changed = true;
}
}
}
}

if (!Changed)
return PreservedAnalyses::all();
PreservedAnalyses PA = PreservedAnalyses::none();
PA.preserve<DominatorTreeAnalysis>();
PA.preserve<PostDominatorTreeAnalysis>();
PA.preserve<LoopAnalysis>();
return PA;
}
1 change: 1 addition & 0 deletions llvm/test/Other/new-pm-thinlto-postlink-defaults.ll
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@
; CHECK-O-NEXT: Invalidating analysis: ShouldNotRunFunctionPassesAnalysis
; CHECK-O-NEXT: Invalidating analysis: InlineAdvisorAnalysis
; CHECK-O-NEXT: Running pass: DeadArgumentEliminationPass
; CHECK-O-NEXT: Running pass: SimplifyTypeTestsPass
; CHECK-O-NEXT: Running pass: CoroCleanupPass
; CHECK-POSTLINK-O-NEXT: Running pass: GlobalOptPass
; CHECK-POSTLINK-O-NEXT: Running pass: GlobalDCEPass
Expand Down
1 change: 1 addition & 0 deletions llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@
; CHECK-O-NEXT: Invalidating analysis: ShouldNotRunFunctionPassesAnalysis
; CHECK-O-NEXT: Invalidating analysis: InlineAdvisorAnalysis
; CHECK-O-NEXT: Running pass: DeadArgumentEliminationPass
; CHECK-O-NEXT: Running pass: SimplifyTypeTestsPass
; CHECK-O-NEXT: Running pass: CoroCleanupPass
; CHECK-O-NEXT: Running pass: GlobalOptPass
; CHECK-O-NEXT: Running pass: GlobalDCEPass
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@
; CHECK-O-NEXT: Invalidating analysis: ShouldNotRunFunctionPassesAnalysis
; CHECK-O-NEXT: Invalidating analysis: InlineAdvisorAnalysis
; CHECK-O-NEXT: Running pass: DeadArgumentEliminationPass
; CHECK-O-NEXT: Running pass: SimplifyTypeTestsPass
; CHECK-O-NEXT: Running pass: CoroCleanupPass
; CHECK-O-NEXT: Running pass: GlobalOptPass
; CHECK-O-NEXT: Running pass: GlobalDCEPass
Expand Down
46 changes: 46 additions & 0 deletions llvm/test/Transforms/SimplifyTypeTests/basic.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
; Test that a lowered type test for a type is simplified to true
; if the target is a constant member of that type.

; RUN: opt -S %s -passes=simplify-type-tests | FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment about what this is testing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


; Test that the simplification does not occur if the type is wrong.

; RUN: sed -e 's/"_ZTSFvvE"/"wrongtype"/g' %s | opt -S -passes=simplify-type-tests | FileCheck --check-prefix=WRONGTYPE %s

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

@__typeid__ZTSFvvE_global_addr = external hidden global [0 x i8], code_model "small"

define void @_Z2fpv.cfi() !type !0 {
ret void
}

define i64 @main() {
%1 = icmp eq ptr @_Z2fpv, @__typeid__ZTSFvvE_global_addr
; CHECK: br i1 true
; WRONGTYPE: br i1 %
br i1 %1, label %3, label %2

2:
tail call void @llvm.ubsantrap(i8 2)
unreachable

3:
; CHECK: br i1 true
; WRONGTYPE: br i1 %
%c = icmp eq i64 ptrtoint (ptr @_Z2fpv to i64), ptrtoint (ptr @__typeid__ZTSFvvE_global_addr to i64)
br i1 %c, label %4, label %2

4:
tail call void @_Z2fpv()
; CHECK: ret i64 0
; WRONGTYPE: ret i64 sub
ret i64 sub (i64 ptrtoint (ptr @_Z2fpv to i64), i64 ptrtoint (ptr @__typeid__ZTSFvvE_global_addr to i64))
}

declare void @llvm.ubsantrap(i8 immarg)

declare hidden void @_Z2fpv()

!0 = !{i64 0, !"_ZTSFvvE"}
Loading