[PATCH] Teach ScalarEvolution to exploit min and max expressions.

Andrew Trick atrick at apple.com
Fri Dec 12 00:34:01 PST 2014


Overall really nice work. Thanks to David for reviewing. See my comments inline.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:6952
@@ +6951,3 @@
+
+/// If Expr computes ~A assign `A' to NotOf and return true, else
+/// return false.
----------------
insert a comma: "computes ~A, assign"

================
Comment at: lib/Analysis/ScalarEvolution.cpp:6954
@@ +6953,3 @@
+/// return false.
+static bool MatchNotExpr(const SCEV *Expr, const SCEV *&NotOf) {
+  const SCEVAddExpr *Add = dyn_cast<SCEVAddExpr>(Expr);
----------------
Not a big deal, but why not return NotOf and nullptr for a mismatch?

Did you implement the pattern matching to avoid the overhead of just comparing the result of getNotExpr if you can bail-out of the match early?

================
Comment at: lib/Analysis/ScalarEvolution.cpp:6983
@@ +6982,3 @@
+  for (const SCEV *Op : MaxExpr->operands())
+    if (SE.getMinusSCEV(Candidate, Op)->isZero())
+      return true;
----------------
Can you explain why the subtraction is necessary. What unique expressions have zero difference?

Are you assuming that MaybeExpr and Candidate have the same bitwidth? What happens if they don't?

================
Comment at: lib/Analysis/ScalarEvolution.cpp:7034
@@ +7033,3 @@
+
+  llvm_unreachable("covered switch fell through?!");
+}
----------------
Out of curiosity, do you need the unreachable? There shouldn't be a warning for a covered switch, should be a warning otherwise.

http://reviews.llvm.org/D6635

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list