[PATCH] D39835: [GVN PRE] Clear nsw/nuw for original values in LoadPRE

[Serguei Katkov via Phabricator via llvm-commits]

skatkov created this revision.

When we construct Phi node in LoadPRE we should clear
nsw/num falgs in all original values. Otherwise it is possible
that we can see the poison value where it is not expected.

The added test shows an example of incorrect behavior of
jump threading pass due to GVN PRE did not clear the flag.

Thank you to Daniel Berlin for test.


https://reviews.llvm.org/D39835

Files:
  lib/Transforms/Scalar/GVN.cpp
  test/Transforms/GVN/PRE/pre-jt-load.ll


Index: test/Transforms/GVN/PRE/pre-jt-load.ll
===================================================================
--- /dev/null
+++ test/Transforms/GVN/PRE/pre-jt-load.ll
@@ -0,0 +1,39 @@
+; RUN: opt < %s -gvn -enable-pre -jump-threading -S | FileCheck %s
+
+ at H = common global i32 0
+ at G = common global i32 0
+
+define i32 @test(i1 %cond, i32 %v) nounwind {
+; CHECK-LABEL: @test
+entry:
+  br i1 %cond, label %bb, label %bb1
+
+bb:
+; CHECK: store
+; CHECK-NOT: br label %return
+  %add.1 = add nuw nsw i32 %v, -1
+  store i32 %add.1, i32* @G, align 4
+  br label %merge
+
+bb1:
+; CHECK: store
+; CHECK-NOT: br label %return
+  %add.2 = add i32 %v, -1
+  store i32 %add.2, i32* @G, align 4
+  br label %merge
+
+merge:
+  %foo = load i32, i32* @G, align 4
+  %cmp = icmp sgt i32 %foo, 0
+  br i1 %cmp, label %action, label %return
+
+action:
+; CHECK: store
+; CHECK-NEXT: br label %return
+  store i32 %foo, i32* @H, align 4
+  br label %return
+
+return:
+  %p = phi i32 [0, %merge], [1, %action]
+  ret i32 %p
+}
Index: lib/Transforms/Scalar/GVN.cpp
===================================================================
--- lib/Transforms/Scalar/GVN.cpp
+++ lib/Transforms/Scalar/GVN.cpp
@@ -765,6 +765,16 @@
     if (SSAUpdate.HasValueForBlock(BB))
       continue;
 
+    // We must clear nuw/nsw for original value. Otherwise we
+    // can see the poison value where it was not originally.
+    Value *V = AV.AV.Val.getPointer();
+    if (V)
+      if (auto *I = dyn_cast<Instruction>(V))
+        if (isa<OverflowingBinaryOperator>(I)) {
+          I->setHasNoSignedWrap(false);
+          I->setHasNoUnsignedWrap(false);
+        }
+
     SSAUpdate.AddAvailableValue(BB, AV.MaterializeAdjustedValue(LI, gvn));
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39835.122214.patch
Type: text/x-patch
Size: 1727 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171109/d95d660d/attachment.bin>