Skip to content
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

release/18.x: [GVN] Drop nsw/nuw flags when replacing the result of a with.overflow intrinsic with a overflowing binary operator (#82935) #82965

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

llvmbot
Copy link
Collaborator

@llvmbot llvmbot commented Feb 26, 2024

Backport 892b4be

Requested by: @dtcxzyw

@llvmbot llvmbot added this to the LLVM 18.X Release milestone Feb 26, 2024
@llvmbot
Copy link
Collaborator Author

llvmbot commented Feb 26, 2024

@nikic What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Collaborator Author

llvmbot commented Feb 26, 2024

@llvm/pr-subscribers-llvm-transforms

Author: None (llvmbot)

Changes

Backport 892b4be

Requested by: @dtcxzyw


Full diff: https://github.com/llvm/llvm-project/pull/82965.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Utils/Local.cpp (+7-1)
  • (added) llvm/test/Transforms/GVN/pr82884.ll (+21)
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 459e3d98059283..a1c6bbc52fd05e 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -3369,11 +3369,17 @@ void llvm::patchReplacementInstruction(Instruction *I, Value *Repl) {
 
   // Patch the replacement so that it is not more restrictive than the value
   // being replaced.
+  WithOverflowInst *UnusedWO;
+  // When replacing the result of a llvm.*.with.overflow intrinsic with a
+  // overflowing binary operator, nuw/nsw flags may no longer hold.
+  if (isa<OverflowingBinaryOperator>(ReplInst) &&
+      match(I, m_ExtractValue<0>(m_WithOverflowInst(UnusedWO))))
+    ReplInst->dropPoisonGeneratingFlags();
   // Note that if 'I' is a load being replaced by some operation,
   // for example, by an arithmetic operation, then andIRFlags()
   // would just erase all math flags from the original arithmetic
   // operation, which is clearly not wanted and not needed.
-  if (!isa<LoadInst>(I))
+  else if (!isa<LoadInst>(I))
     ReplInst->andIRFlags(I);
 
   // FIXME: If both the original and replacement value are part of the
diff --git a/llvm/test/Transforms/GVN/pr82884.ll b/llvm/test/Transforms/GVN/pr82884.ll
new file mode 100644
index 00000000000000..71abafda60d93d
--- /dev/null
+++ b/llvm/test/Transforms/GVN/pr82884.ll
@@ -0,0 +1,21 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -S -passes=gvn < %s | FileCheck %s
+
+; Make sure nsw/nuw flags are dropped.
+
+define i32 @pr82884(i32 %x) {
+; CHECK-LABEL: define i32 @pr82884(
+; CHECK-SAME: i32 [[X:%.*]]) {
+; CHECK-NEXT:    [[MUL:%.*]] = mul i32 [[X]], [[X]]
+; CHECK-NEXT:    call void @use(i32 [[MUL]])
+; CHECK-NEXT:    [[MUL2:%.*]] = call { i32, i1 } @llvm.smul.with.overflow.i32(i32 [[X]], i32 [[X]])
+; CHECK-NEXT:    ret i32 [[MUL]]
+;
+  %mul = mul nsw nuw i32 %x, %x
+  call void @use(i32 %mul)
+  %mul2 = call { i32, i1 } @llvm.smul.with.overflow.i32(i32 %x, i32 %x)
+  %ret = extractvalue { i32, i1 } %mul2, 0
+  ret i32 %ret
+}
+
+declare void @use(i32)

… intrinsic with a overflowing binary operator (llvm#82935)

Alive2: https://alive2.llvm.org/ce/z/gyL7mn
Fixes llvm#82884.

(cherry picked from commit 892b4be)
@tstellar tstellar merged commit 3aea3d2 into llvm:release/18.x Feb 26, 2024
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants