[PATCH] D30350: [LSR] Add a cap for reassociation of AllFixupsOutsideLoop type LSRUse to protect compile time

Wei Mi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 10 14:11:20 PDT 2017


wmi added a comment.

Sanjoy, thanks for the comments.

> Is it possible to solve this by adding a cache that maps SCEV expressions to their zero (and sign) extended variants? We'd not want this cache to be permanent, since it can lock SCEV into a pessimistic state that it won't get out of even after (say) proving a value to be nuw. However, we can create one such cache in the lexical scope of the top level getZeroExtendExpr (and maybe split out a getZeroExtendImpl that takes a pointer to said cache).

It sounds a good solution. I will try it if my simple fix is still not enough after adding the constraint.



================
Comment at: lib/Analysis/ScalarEvolution.cpp:1790
 
+      // If we have special knowledge that this addrec won't self-overflow,
+      // we don't need to do any further analysis.
----------------
sanjoy wrote:
> I'm not sure if this is correct.  You're saying that, e.g., `sext({A,+,B}<nw>` is `{sext A,+,zext B}`, but what if `A` is `INT_MAX`, `B` is `1`, and the backedge taken count count is 1?
Is adding one more constraint isKnownNegative(Step) enough?


================
Comment at: unittests/Analysis/ScalarEvolutionTest.cpp:625
+      "for.cond1:\n"
+      "  %i2.0 = phi i64 [ %inc5, %for.inc4 ], [ 100, %for.cond ]\n"
+      "  %cmp2 = icmp sgt i64 %i2.0, 90\n"
----------------
sanjoy wrote:
> Did you consider generating this IR programmatically?  That would be easier to modify.
Ok, I will do it.


Repository:
  rL LLVM

https://reviews.llvm.org/D30350





More information about the llvm-commits mailing list