[PATCH] D121452: [VPlan] Avoid scalarization for scalable vectors.

[Malhar via Phabricator via llvm-commits]

malharJ added inline comments.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-avoid-scalarization.ll:5
+
+; CHECK-NOT: Assertion {{.*}} "Can't scalarize a scalable vector"' failed
+
----------------
fhahn wrote:
> Does this fail without the change or does this need `2>&1`?
this test will fail without this change .. I dont think 2>&1 is needed.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-avoid-scalarization.ll:23
+  %1 = getelementptr i64, i64* %a, i32 %indvars.iv
+  %2 = bitcast i64* %1 to double*
+  %3 = load double, double* %2, align 8
----------------
fhahn wrote:
> Is the bit cast needed or could this just load `i64`? It would also be good to add make sure the load is not dead in the loop.
the bitcast is actually needed.
It results in the gep not being classified as 'uniform' which is required for this test case to generate the assert failure (without this patch).

(the way the `collectLoopUniforms()` logic works is that it looks at the load/store
and places it's pointer operand (which is the bitcast here) into a worklist. 
It then iterates over the bitcast's operands (which in this case is the gep) and checks for uniformity. And since the gep has one usage outside the loop, it is marked as not uniform. This consequently results in the loop induction update variable being marked as not uniform, but as a scalar (REPLICATE recipe) causing the assertion failure)


And regarding the dead load, I was just trying to create a minimal testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121452