-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Add SimplifyTypeTests pass. #141327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SimplifyTypeTests pass. #141327
Changes from all commits
ab837f0
d7bb3c4
1344339
16d5799
b36c74c
b76d0d4
abfa556
d140ba4
7c0b3ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
|
@@ -2472,3 +2475,94 @@ 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 | ||
// 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(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would just use There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
}; | ||
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; | ||
} |
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a comment about what this is testing There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done