[llvm] c0cc982 - [Float2Int] Make sure dependent ranges are calculated first (PR54669)

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2022-04-04T10:18:39+02:00
New Revision: c0cc98251a453e2850c25adb10a3fd6b3e86a2e8

URL: https://github.com/llvm/llvm-project/commit/c0cc98251a453e2850c25adb10a3fd6b3e86a2e8
DIFF: https://github.com/llvm/llvm-project/commit/c0cc98251a453e2850c25adb10a3fd6b3e86a2e8.diff

LOG: [Float2Int] Make sure dependent ranges are calculated first (PR54669)

The range calculation in walkForwards() assumes that the ranges of
the operands have already been calculated. With the used visit
order, this is not necessarily the case when there are multiple
roots. (There is nothing guaranteeing that instructions are visited
in topological order.)

Fix this by queuing instructions for reprocessing if the operand
ranges haven't been calculated yet.

Fixes https://github.com/llvm/llvm-project/issues/54669.

Differential Revision: https://reviews.llvm.org/D122817

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Scalar/Float2Int.h
    llvm/lib/Transforms/Scalar/Float2Int.cpp
    llvm/test/Transforms/Float2Int/pr54669.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Scalar/Float2Int.h b/llvm/include/llvm/Transforms/Scalar/Float2Int.h
index 9def40ed880d4..f4bec228ea963 100644
--- a/llvm/include/llvm/Transforms/Scalar/Float2Int.h
+++ b/llvm/include/llvm/Transforms/Scalar/Float2Int.h
@@ -25,6 +25,7 @@ class DominatorTree;
 class Function;
 class Instruction;
 class LLVMContext;
+template <typename T> class Optional;
 class Type;
 class Value;
 
@@ -41,7 +42,7 @@ class Float2IntPass : public PassInfoMixin<Float2IntPass> {
   ConstantRange badRange();
   ConstantRange unknownRange();
   ConstantRange validateRange(ConstantRange R);
-  ConstantRange calcRange(Instruction *I);
+  Optional<ConstantRange> calcRange(Instruction *I);
   void walkBackwards();
   void walkForwards();
   bool validateAndTransform();

diff  --git a/llvm/lib/Transforms/Scalar/Float2Int.cpp b/llvm/lib/Transforms/Scalar/Float2Int.cpp
index 6b04d2ce07324..d781cc0ab19df 100644
--- a/llvm/lib/Transforms/Scalar/Float2Int.cpp
+++ b/llvm/lib/Transforms/Scalar/Float2Int.cpp
@@ -235,14 +235,17 @@ void Float2IntPass::walkBackwards() {
   }
 }
 
-// Calculate result range from operand ranges
-ConstantRange Float2IntPass::calcRange(Instruction *I) {
+// Calculate result range from operand ranges.
+// Return None if the range cannot be calculated yet.
+Optional<ConstantRange> Float2IntPass::calcRange(Instruction *I) {
   SmallVector<ConstantRange, 4> OpRanges;
   for (Value *O : I->operands()) {
     if (Instruction *OI = dyn_cast<Instruction>(O)) {
-      assert(SeenInsts.find(OI) != SeenInsts.end() &&
-             "def not seen before use!");
-      OpRanges.push_back(SeenInsts.find(OI)->second);
+      auto OpIt = SeenInsts.find(OI);
+      assert(OpIt != SeenInsts.end() && "def not seen before use!");
+      if (OpIt->second == unknownRange())
+        return None; // Wait until operand range has been calculated.
+      OpRanges.push_back(OpIt->second);
     } else if (ConstantFP *CF = dyn_cast<ConstantFP>(O)) {
       // Work out if the floating point number can be losslessly represented
       // as an integer.
@@ -324,12 +327,19 @@ ConstantRange Float2IntPass::calcRange(Instruction *I) {
 // Walk forwards down the list of seen instructions, so we visit defs before
 // uses.
 void Float2IntPass::walkForwards() {
-  for (auto &It : reverse(SeenInsts)) {
-    if (It.second != unknownRange())
-      continue;
+  std::deque<Instruction *> Worklist;
+  for (const auto &Pair : SeenInsts)
+    if (Pair.second == unknownRange())
+      Worklist.push_back(Pair.first);
+
+  while (!Worklist.empty()) {
+    Instruction *I = Worklist.back();
+    Worklist.pop_back();
 
-    Instruction *I = It.first;
-    seen(I, calcRange(I));
+    if (Optional<ConstantRange> Range = calcRange(I))
+      seen(I, *Range);
+    else
+      Worklist.push_front(I); // Reprocess later.
   }
 }
 

diff  --git a/llvm/test/Transforms/Float2Int/pr54669.ll b/llvm/test/Transforms/Float2Int/pr54669.ll
index 135bee55fe260..f56040c14d577 100644
--- a/llvm/test/Transforms/Float2Int/pr54669.ll
+++ b/llvm/test/Transforms/Float2Int/pr54669.ll
@@ -3,11 +3,9 @@
 
 declare void @use(i32)
 
-; FIXME: Currently being miscompiled.
-
 define i1 @src() {
 ; CHECK-LABEL: @src(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 2147483647, -1
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 -1, -1
 ; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %add = fadd double 2.000000e+00, -1.000000e+00