-
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
base: users/pcc/spr/main.add-simplifytypetests-pass
Are you sure you want to change the base?
Add SimplifyTypeTests pass. #141327
Changes from all commits
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,6 +19,7 @@ | |
#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" | ||
|
@@ -2478,3 +2479,76 @@ 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. | ||
for (auto &GV : M.globals()) { | ||
if (!GV.getName().starts_with("__typeid_") || | ||
!GV.getName().ends_with("_global_addr")) | ||
continue; | ||
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 |
||
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. |
||
}; | ||
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(); | ||
return PreservedAnalyses::none(); | ||
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. Is that true? E.g. the CFG doesn't change, right? Could we do what we do in HWASan: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L485? |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
; 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 |
||
; 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.