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

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: uabelho, spatel, reames.
Herald added a subscriber: hiraditya.
Herald added a project: All.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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.


https://reviews.llvm.org/D122817

Files:
  llvm/include/llvm/Transforms/Scalar/Float2Int.h
  llvm/lib/Transforms/Scalar/Float2Int.cpp


Index: llvm/lib/Transforms/Scalar/Float2Int.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/Float2Int.cpp
+++ llvm/lib/Transforms/Scalar/Float2Int.cpp
@@ -235,14 +235,17 @@
   }
 }
 
-// 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 @@
 // 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.
   }
 }
 
Index: llvm/include/llvm/Transforms/Scalar/Float2Int.h
===================================================================
--- llvm/include/llvm/Transforms/Scalar/Float2Int.h
+++ llvm/include/llvm/Transforms/Scalar/Float2Int.h
@@ -25,6 +25,7 @@
 class Function;
 class Instruction;
 class LLVMContext;
+template <typename T> class Optional;
 class Type;
 class Value;
 
@@ -41,7 +42,7 @@
   ConstantRange badRange();
   ConstantRange unknownRange();
   ConstantRange validateRange(ConstantRange R);
-  ConstantRange calcRange(Instruction *I);
+  Optional<ConstantRange> calcRange(Instruction *I);
   void walkBackwards();
   void walkForwards();
   bool validateAndTransform();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D122817.419450.patch
Type: text/x-patch
Size: 2616 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220331/832886c6/attachment.bin>