[PATCH] [SCEV][LoopVectorize] Allow ScalarEvolution to make assumptions about overflows

[Andrew Trick]

Overall, it's great to see something like this working in practice. In the past, several alternatives have been discussed. IIRC the main objection to this approach is that SCEV could make assumptions that are not necessary for the optimization leading to overhead for unnecessary checks.

Given this is a major design decision, it would be good to have feedback from anyone who has done a lot of work in the area (thanks to Hal for the review). What about Sanjoy, Arnold (especially for the vectorizer change), MichaelZ, AdamN ??

I don't see a major problem with this approach. Just a few comments on the implementation...

I'm afraid that iterating over a DenseMap will produce SCEV expressions and IR checks in a nondeterministic order. We usually fix this with a MapVector.

+    AssumptionMapTy::iterator getLoopAssumptionBegin(const Loop *L) {

I don't see AnalyzedLoop being initialized in the ctor:

+ AssumingScalarEvolution::AssumingScalarEvolution() :
+   ScalarEvolution(false, ID), SE(nullptr) {

See Instruction::getModule()

+  Module *M = Loc->getParent()->getParent()->getParent();

On most targets, I think it's more efficient to check the high bits for overflow. e.g.

  if ((a + 0x800000) & 0xffffff000000) overflow

Instead of:

+    if (Signed) {
+      APInt CmpMinValue = APInt::getSignedMinValue(DstBits).sext(SrcBits);
+      ConstantInt *CTMin = ConstantInt::get(M->getContext(), CmpMinValue);
+      Value *MinCheck = OFBuilder.CreateICmp(ICmpInst::ICMP_SLT,
+                                             TripCount,
+                                             CTMin);
+      TripCountCheck = OFBuilder.CreateOr(TripCountCheck, MinCheck);
+    }


http://reviews.llvm.org/D10161

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