[PATCH] D114155: [LoopAccessAnalysis][SVE] Bail out for scalable vectors

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen accepted this revision.
sdesmalen added inline comments.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1106
+  if (AllocSize.isScalable()) {
+    // Scalable types not yet supported - give up.
+    return 0;
----------------
peterwaller-arm wrote:
> sdesmalen wrote:
> > I think there are three possible scenarios to consider:
> > 
> > 1. The pointer has a scalable stride (e.g. the PHI is updated with `getetelemenptr <vscale x 2 x i64>, <vscale x 2 x i64>*, i64 1` each iteration of the loop), at which point the Step will not be a SCEVConstant, but rather a SCEVUnknown.
> > 2. Case 1, but LAA has replaced the symbolic stride by 1 under the predicate that vscale = 1.
> > 3. The case you have in your tests, where the pointer stride is actually fixed (e.g. Step = `{nullptr, +, 1}`) and  the object is scalable.
> > 
> > For case 1, the dyn_cast on line 1096 would fail. That means the code is already conservative.
> > For case 2, we could do some extra work to recognize there is a predicate that checks that vscale == 1, and use `getKnownMinSize()` and allow the code-path here to continue.
> > For case 3, we can bail out and no further work is required (meaning that the 'yet' can be removed from the comment).
> > 
> > Sorry if I'm being a bit nitpicky, but I think the 'not yet supported' is already the case today, and your comment is a bit misleading in that the 'yet' at this point in the code only applies to 2.
> > 
> > Maybe it's worth capturing these scenarios somewhere in a comment?
> Looking at it, I don't think I observed the dyn_cast on 1096 fail. So with respect to your request I'm uncertain exactly what I am documenting right now (all the scalable cases I've tried appear to flow through the new debug statement) and I've chosen to leave that out. Hope that's OK but can revisit if requested.
I'd expect that `dyn_cast` to fail if there's a bitcast in the loop:
```  %ind_ptr_i8 = bitcast <vscale x 16 x i8>* %ind_ptr to i8*
  store i8 0, i8* %ind_ptr_i8
  %next_ptr = getelementptr inbounds <vscale x 16 x i8>, <vscale x 16 x i8>* %ind_ptr, i64 1```

That would create a SCEV with scalable stride: `{%start,+,sizeof(<vscale x 16 x i8>)}`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114155/new/

https://reviews.llvm.org/D114155