-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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
[CodeGenPrepare] Preserve flags (such as nsw/nuw) in SinkCast #89904
[CodeGenPrepare] Preserve flags (such as nsw/nuw) in SinkCast #89904
Conversation
@llvm/pr-subscribers-llvm-transforms Author: Alex Bradbury (asb) ChangesAs demonstrated in the test change, when deciding to sink a trunc we were losing its flags. This patch moves to cloning the original instruction instead. CC @elhewaty Full diff: https://github.com/llvm/llvm-project/pull/89904.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 22a766f8d62524..be13db41ce5e66 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -1431,10 +1431,8 @@ static bool SinkCast(CastInst *CI) {
if (!InsertedCast) {
BasicBlock::iterator InsertPt = UserBB->getFirstInsertionPt();
assert(InsertPt != UserBB->end());
- InsertedCast = CastInst::Create(CI->getOpcode(), CI->getOperand(0),
- CI->getType(), "");
+ InsertedCast = static_cast<CastInst *>(CI->clone());
InsertedCast->insertBefore(*UserBB, InsertPt);
- InsertedCast->setDebugLoc(CI->getDebugLoc());
}
// Replace a use of the cast with a use of the new cast.
diff --git a/llvm/test/Transforms/CodeGenPrepare/RISCV/noop-copy-sink.ll b/llvm/test/Transforms/CodeGenPrepare/RISCV/noop-copy-sink.ll
new file mode 100644
index 00000000000000..55cde6c1431fe3
--- /dev/null
+++ b/llvm/test/Transforms/CodeGenPrepare/RISCV/noop-copy-sink.ll
@@ -0,0 +1,30 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes='require<profile-summary>,function(codegenprepare)' -mtriple=riscv64 %s \
+; RUN: | FileCheck --check-prefixes=CHECK %s
+
+define i16 @sink_trunc1(i64 %a) {
+; CHECK-LABEL: @sink_trunc1(
+; CHECK-NEXT: fnend:
+; CHECK-NEXT: [[TMP0:%.*]] = trunc i64 [[A:%.*]] to i16
+; CHECK-NEXT: ret i16 [[TMP0]]
+;
+ %trunc = trunc i64 %a to i16
+ br label %fnend
+
+fnend:
+ ret i16 %trunc
+}
+
+; The flags on the original trunc should be preserved.
+define i16 @sink_trunc2(i64 %a) {
+; CHECK-LABEL: @sink_trunc2(
+; CHECK-NEXT: fnend:
+; CHECK-NEXT: [[TMP0:%.*]] = trunc nuw nsw i64 [[A:%.*]] to i16
+; CHECK-NEXT: ret i16 [[TMP0]]
+;
+ %trunc = trunc nuw nsw i64 %a to i16
+ br label %fnend
+
+fnend:
+ ret i16 %trunc
+}
|
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.
LGTM
…sw/nuw are dropped SinkCast creates a new cast with the same type and inputs, which drops the nsw/nuw flags. Reviewed as part of <#89904> but split out so I can land the test separately.
As demonstrated in the test change, when deciding to sink a trunc we were losing its flags. This patch moves to cloning the original instruction instead.
b812621
to
a242885
Compare
As demonstrated in the test change, when deciding to sink a trunc we were losing its flags. This patch moves to cloning the original instruction instead.
CC @elhewaty