[PATCH] D95139: [SVE][LoopVectorize] Add support for extracting the last lane of a scalable vector

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2486
 
-  assert(Instance.Lane > 0
+  assert(!Instance.isFirstLane()
              ? !Cost->isUniformAfterVectorization(cast<Instruction>(V), VF)
----------------
sdesmalen wrote:
> nit: Can you maybe write this as `Instance.Lane.isFirst()`? Personally I find the difference between isFirstLane and isFirstIteration a bit confusing. (same for other places)
Sure. I added isFirstIteration at your suggestion because it refers to Part=0 as well as Lane=0.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:70
+    break;
+  default:
+    assert(LaneKind == VPLane::Kind::First);
----------------
sdesmalen wrote:
> don't use default. Cover both cases explicitly, so that if another enum value is added, the compiler will emit a diagnostic this case is not covered.
OK I can do that - it just might mean adding an initialiser to Lane at the start of the function. I can't return directly from a case statement without a default as the compiler warns about functions returning void otherwise.


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

https://reviews.llvm.org/D95139