[ValueTracking] Fix isSafeToSpeculativelyExecute for sdiv (PR48778)

The != -1 check does not work correctly for all bitwidths. Use isAllOnesValue() instead.
checkedc · Jan 17, 2021 · 4229b87 · 4229b87
1 parent 1cc477f
commit 4229b87
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 4 deletions.
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
@@ -4391,7 +4391,7 @@ bool llvm::isSafeToSpeculativelyExecute(const Value *V,
     if (*Denominator == 0)
       return false;
     // It's safe to hoist if the denominator is not 0 or -1.
-    if (*Denominator != -1)
+    if (!Denominator->isAllOnesValue())
       return true;
     // At this point we know that the denominator is -1.  It is safe to hoist as
     // long we know that the numerator is not INT_MIN.

diff --git a/llvm/test/Transforms/SimplifyCFG/pr48778-sdiv-speculation.ll b/llvm/test/Transforms/SimplifyCFG/pr48778-sdiv-speculation.ll
@@ -4,11 +4,14 @@
 ; sdiv INT_MIN / -1 should not be speculated.
 define i32 @test(i1 %cmp) {
 ; CHECK-LABEL: @test(
-; CHECK-NEXT:  else:
+; CHECK-NEXT:    br i1 [[CMP:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
+; CHECK:       if:
 ; CHECK-NEXT:    [[DIV:%.*]] = sdiv i32 -2147483648, -1
 ; CHECK-NEXT:    [[CMP2:%.*]] = icmp ne i32 [[DIV]], 0
-; CHECK-NEXT:    [[OR_COND:%.*]] = and i1 [[CMP:%.*]], [[CMP2]]
-; CHECK-NEXT:    [[MERGE:%.*]] = select i1 [[OR_COND]], i32 1, i32 0
+; CHECK-NEXT:    [[SPEC_SELECT:%.*]] = select i1 [[CMP2]], i32 1, i32 0
+; CHECK-NEXT:    br label [[ELSE]]
+; CHECK:       else:
+; CHECK-NEXT:    [[MERGE:%.*]] = phi i32 [ 0, [[TMP0:%.*]] ], [ [[SPEC_SELECT]], [[IF]] ]
 ; CHECK-NEXT:    ret i32 [[MERGE]]
 ;
   br i1 %cmp, label %if, label %else