[PATCH] D89692: [SCEV] SCEVPtrToIntExpr simplifications

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:1048
+  assert(!isa<SCEVConstant>(Op) && !isa<SCEVCastExpr>(Op) &&
+         (isa<SCEVNAryExpr>(Op) || isa<SCEVUnknown>(Op)) &&
+         "We can't get a constant or a cast here as an input, "
----------------
If you check it's either Nary or unknown, checking it NOT be something else in addition is an overkill. :)


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:1058
+  if (const SCEVNAryExpr *NaryExpr = dyn_cast<SCEVNAryExpr>(Op)) {
+    SmallVector<const SCEV *, 2> NewOps;
+    NewOps.reserve(NaryExpr->getNumOperands());
----------------
nit: could go with `SmallVector<const SCEV *, 2> NewOps(NaryExpr->getNumOperands())`


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:1064
+                           : Op);
+    const SCEV *NewNaryExpr;
+    switch (SCEVTypes SCEVType = NaryExpr->getSCEVType()) {
----------------
Please init with nullptr to make it easier to track further possible mistakes.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:1085
+    case scUDivExpr:
+    // NewNaryExpr = getUDivExpr(NewOps[0], NewOps[1]);
+    // break;
----------------
Is it commented on purpose?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89692