[PATCH] D40369: Support sext, zext and trunc instructions in SCEV delinearization algorithm (new revision)

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:945-955
+    if (const SCEVTruncateExpr *TruncDenominator =
+                                      dyn_cast<SCEVTruncateExpr>(Denominator)) {
+      divide(SE, CENumerator->getOperand(), TruncDenominator->getOperand(), Q,
+             R);
+    }
+    else
+    if (const SCEVZeroExtendExpr *ZExtDenominator =
----------------
This patch is for handling `sext` only, correct? In this case, are these two cases needed?


================
Comment at: lib/Analysis/ScalarEvolution.cpp:1083
     RewriteMap[cast<SCEVUnknown>(Denominator)->getValue()] =
-        cast<SCEVConstant>(Zero)->getValue();
+        cast<SCEVConstant>(SE.getZero(Denominator->getType()))->getValue();
     Remainder = SCEVParameterRewriter::rewrite(Numerator, SE, RewriteMap, true);
----------------
This change looks unnecessary/unrelated to this patch.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:1089
       RewriteMap[cast<SCEVUnknown>(Denominator)->getValue()] =
-          cast<SCEVConstant>(One)->getValue();
+          cast<SCEVConstant>(SE.getOne(Denominator->getType()))->getValue();
       Quotient =
----------------
This change looks unnecessary/unrelated to this patch.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:1111-1112
       : SE(S), Denominator(Denominator) {
-    Zero = SE.getZero(Denominator->getType());
-    One = SE.getOne(Denominator->getType());
+    Zero = SE.getZero(Numerator->getType());
+    One = SE.getOne(Numerator->getType());
 
----------------
This change looks unnecessary/unrelated to this patch.


================
Comment at: test/Analysis/Delinearization/test_sext.ll:4
+
+; AddRec: {(4 * (sext i32 {{0,+,(%N * %N)}<%for.body3.lr.ph.split.us.us>,+,%N}<%for.body3.us.us> to i64)),+,4}<nsw><%for.body6.us.us>
+; CHECK: Expr: {(4 * (sext i32 {{0,+,(%N * %N)}
----------------
Is this a line to be checked? Then the line must start with `CHECK:` instead of `AddRec:`


Repository:
  rL LLVM

https://reviews.llvm.org/D40369