[PATCH] D17201: [SCEV] Introduce a guarded backedge taken count and use it in LAA and LV

silviu.baranga@arm.com via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 22 03:17:20 PST 2016


sbaranga added a comment.

In http://reviews.llvm.org/D17201#357709, @sanjoy wrote:

> Given that we're now computing predicated trip counts (the bread and butter of SCEV), I think this is good time to start doing some whitebox testing.  What I mean by that is to add a way to get SCEV to produce a predicated trip count and dump both the predicate and the trip count, and do llvm-lit/FileCheck type testing on the output.  Unfortunately, this will be more work for you, but I think this the right thing to do for the system.  We don't want to end up in a situation where adding a test for a predicated trip count related bug involves tricking the loop vectorizer to vectorize a certain loop.
>
> In fact, I think there is already some scope for whitebox testing things like `PredicatedScalarEvolution::getAsAddRec`, maybe that's a good place to start?
>
> Again, I know this is more work for you; but having the infrastructure to easily add tests is important.


Thanks, Sanjoy! I agree, we should do as much testing as possible for this. I'll add the tests.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:5683
@@ -5607,1 +5682,3 @@
+  // Try to compute it using assumptions.
+  return computeExitLimitFromICmp(L, ExitCond, TBB, FBB, ControlsExit, true);
 }
----------------
sanjoy wrote:
> I don't think this is the right layering -- for instance, this forces SCEV clients that don't want speculative / predicated trip counts to pay to cost of computing them.
> 
> I'd say SCEV users that care about predicated trip counts should do this retry themselves, i.e. something like
> 
> ```
> const SCEV *TC = getBackedgeTakenCount(L);
> if (TC is SCEVCouldNotCompute) {
>   SE->forgetLoop(L);
>   TC = getPredicatedBackedgeTakenCount(L);
> }
> ```
> 
Good point. I'll look into how we could compute this more lazily.

Why do you think we would need a SE->forgetLoop() here? FWIW I'm generally trying to avoid having to invalidate the analysis for a given loop.


http://reviews.llvm.org/D17201





More information about the llvm-commits mailing list