[PATCH] D17080: [LAA] Allow more run-time alias checks by coercing pointer expressions to AddRecExprs

[silviu.baranga@arm.com via Phabricator via llvm-commits]

sbaranga added a comment.

Hi Anna,

In https://reviews.llvm.org/D17080#869566, @anna wrote:

> Hi Silviu, 
>  I came across the similar gap, but I aggressively tried converting the PtrSCEV to AddRec in `hasComputableBounds`, which worked on our internal benchmark. Could you please clarify why you have the `Assume` to be false in the first call, i.e. how do we know that it's not useful to try and convert PtrSCEV to AddRec?
>
> This was my patch over the original code.
>
>   diff --git a/lib/Analysis/LoopAccessAnalysis.cpp b/lib/Analysis/LoopAccessAnalysis.cpp
>   index d2dbecd..a1c7584 100644
>   --- a/lib/Analysis/LoopAccessAnalysis.cpp
>   +++ b/lib/Analysis/LoopAccessAnalysis.cpp
>   @@ -608,6 +608,11 @@ static bool hasComputableBounds(PredicatedScalarEvolution &PSE,
>        return true;
>   
>      const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(PtrScev);
>   +  // Sometimes the PtrSCEV can be converted into the AddRec, try to retrieve it
>   +  // if possible.
>   +  if (!AR)
>   +    AR = PSE.getAsAddRec(Ptr);
>   +    if (!AR)
>            return false;
>


We only need to convert to an AddRec if we need to emit the runtime checks (see the comment above the original logic for computing NeedRTCheck).

One example would be if we only have reads in the loop. If we convert in hasComputableBounds we end up having unnecessary versioning (at least with regards to the dependence analysis).

Would the current solution also work in your internal benchmark? If not it might be because something else would need the expression to be an AddRec?

Thanks,
Silviu


https://reviews.llvm.org/D17080