[PATCH] D65845: [SCEV][NFC] evaluateAtIteration may produce incorrect result if accuracy of evalutation point is not enough.

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:1209
+
+  // 'It' should have at least 'CalculationBits' precision for the calculation
+  // to be correct. Check that by verifying that downconversion of upconverted
----------------
Ok, I'm definitely not getting something here.

trunc(zext(y, N), M) is always equal to y when N > M and M is width of y.  The zext adds zeros, then the truncate removes them.

Additionally, the comments above in the intro to this function seem to directly contradict the newly added code.

There's no test case provided to show a difference in output.  

Given all of the above, I'm definitely unconvinced of this patch.  


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:8332
       // Then, evaluate the AddRec.
-      return AddRec->evaluateAtIteration(BackedgeTakenCount, *this);
+      const SCEV *ValAtIter =
+          AddRec->evaluateAtIteration(BackedgeTakenCount, *this);
----------------
This snippet is fine to land separately.  This can already happen K > 1000.


================
Comment at: llvm/lib/Analysis/ScalarEvolutionExpander.cpp:1630
   const SCEV *V = cast<SCEVAddRecExpr>(NewS)->evaluateAtIteration(IH, SE);
-  //cerr << "Evaluated: " << *this << "\n     to: " << *V << "\n";
+  assert(!isa<SCEVCouldNotCompute>(V) &&
+         "Expansion of AddRec failed. Can't evaluate SCEV at iteration.");
----------------
This line too for same reason.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65845/new/

https://reviews.llvm.org/D65845