-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[InstCombine] Call InstSimplify for cast instructions #162849
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: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesInstCombine currently fails to call into InstSimplify for cast instructions. I noticed this because the transform from #98649 can be triggered via Full diff: https://github.com/llvm/llvm-project/pull/162849.diff 3 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
index 4c9b10a094981..cdc559b489e9d 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -156,9 +156,9 @@ Instruction *InstCombinerImpl::commonCastTransforms(CastInst &CI) {
Value *Src = CI.getOperand(0);
Type *Ty = CI.getType();
- if (auto *SrcC = dyn_cast<Constant>(Src))
- if (Constant *Res = ConstantFoldCastOperand(CI.getOpcode(), SrcC, Ty, DL))
- return replaceInstUsesWith(CI, Res);
+ if (Value *Res =
+ simplifyCastInst(CI.getOpcode(), Src, Ty, SQ.getWithInstruction(&CI)))
+ return replaceInstUsesWith(CI, Res);
// Try to eliminate a cast of a cast.
if (auto *CSrc = dyn_cast<CastInst>(Src)) { // A->B->C cast
diff --git a/llvm/test/Transforms/InstCombine/cast-set-preserve-signed-dbg-val.ll b/llvm/test/Transforms/InstCombine/cast-set-preserve-signed-dbg-val.ll
index 7cc4446f1038b..ad45d1e3e3e4f 100644
--- a/llvm/test/Transforms/InstCombine/cast-set-preserve-signed-dbg-val.ll
+++ b/llvm/test/Transforms/InstCombine/cast-set-preserve-signed-dbg-val.ll
@@ -11,10 +11,8 @@ define i16 @test5(i16 %A) !dbg !34 {
call void @llvm.dbg.value(metadata i32 %C, metadata !37, metadata !DIExpression()), !dbg !41
; Preserve the dbg.value for the DCE'd 32-bit 'and'.
- ;
- ; The high 16 bits of the original 'and' require sign-extending the new 16-bit and:
; CHECK-NEXT: #dbg_value(i16 [[and]], [[C:![0-9]+]],
- ; CHECK-SAME: !DIExpression(DW_OP_LLVM_convert, 16, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value)
+ ; CHECK-SAME: !DIExpression(DW_OP_LLVM_convert, 16, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_stack_value)
%D = trunc i32 %C to i16, !dbg !42
call void @llvm.dbg.value(metadata i16 %D, metadata !38, metadata !DIExpression()), !dbg !42
diff --git a/llvm/test/Transforms/InstCombine/ptr-int-cast.ll b/llvm/test/Transforms/InstCombine/ptr-int-cast.ll
index 69b8f6953d61e..82ecbd41e50a7 100644
--- a/llvm/test/Transforms/InstCombine/ptr-int-cast.ll
+++ b/llvm/test/Transforms/InstCombine/ptr-int-cast.ll
@@ -86,3 +86,14 @@ define <4 x ptr> @test7(<4 x i128> %arg) nounwind {
%p1 = inttoptr <4 x i128> %arg to <4 x ptr>
ret <4 x ptr> %p1
}
+
+define i64 @ptrtoint_gep_sub(ptr %ptr, i64 %end.addr) {
+; CHECK-LABEL: @ptrtoint_gep_sub(
+; CHECK-NEXT: ret i64 [[END_ADDR:%.*]]
+;
+ %ptr.addr = ptrtoint ptr %ptr to i64
+ %size = sub i64 %end.addr, %ptr.addr
+ %end = getelementptr i8, ptr %ptr, i64 %size
+ %end.addr2 = ptrtoint ptr %end to i64
+ ret i64 %end.addr2
+}
|
; The high 16 bits of the original 'and' require sign-extending the new 16-bit and: | ||
; CHECK-NEXT: #dbg_value(i16 [[and]], [[C:![0-9]+]], | ||
; CHECK-SAME: !DIExpression(DW_OP_LLVM_convert, 16, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value) | ||
; CHECK-SAME: !DIExpression(DW_OP_LLVM_convert, 16, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_stack_value) |
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.
My reading of this DIExpression is that we're first extending to 32-bit and then truncation to 16-bit, in which case the signedness does not matter. But I'm not confident on that reading...
The difference in behavior is that we're directly removing a cast pair instead of converting it into a bitcast first, which has some special handling here, though I have no idea what it's doing: https://github.com/nikic/llvm-project/blob/087ad0bea2e991d6c6b546170994cb2d172a6261/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp#L169-L171
InstCombine currently fails to call into InstSimplify for cast instructions. I noticed this because the transform from #98649 can be triggered via
-passes=instsimplify
but not-passes=instcombine
, which is not supposed to happen.