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

[Peter Waller via Phabricator via llvm-commits]

peterwaller-arm added inline comments.


================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/scalable-vector-regression-tests.ll:22
+define i1 @regression_test_scalable_loop_access_typesize() {
+  br i1 false, label %2, label %1
+1:
----------------
david-arm wrote:
> Is it possible to clean this test up a bit here?
> 
> i.e. do something like:
> 
>   entry:
>     br label %vector.body
> 
>   vector.body:
>     %ind = phi i64 [ 0, %entry ], [ %ind.next, %vector.body ]
>     ...
>     %ind.next = add i64, %ind, 1
>     br i1 %cmp, %vector.body, %end
> 
>   end:
>     ret i1 false
> 
> I'm just a bit worried about the strength of the test given the `br i1 false` and `br i1 true` lines in here. Also, having proper named labels like `vector.body` or `loop.body` makes it a bit more readable I think.
Thanks for your nice suggestions.

> I'm just a bit worried about the strength of the test given the `br i1 false` and `br i1 true` lines in here.

I tried feeding in an i1 %cond parameter, and branching conditionally on that parameter, but then the testcase fails to reproduce the issue. I also tried having the branch depend on the output of the load, and storing an unguessable value from a parameter, but that didn't work either.

Tempted to leave it as br i1 true for the vector body, unless you strongly counsel otherwise; but I will first try a bit harder to determine why the desired code paths aren't hit in that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114155