[PATCH] D101260: [LoopVectorize][SVE] Remove assert for scalable vector in InnerLoopVectorizer::fixReduction

[David Sherwood via Phabricator via llvm-commits]

david-arm accepted this revision.
david-arm added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks for updating the tests - I think it's a stronger test now with the PHIs and makes it clearer what's going on. :)



================
Comment at: llvm/test/Transforms/LoopVectorize/sve-reduction-inloop.ll:10
+; CHECK-NEXT:    [[INDEX:%.*]] = phi i32 [ 0, {{%.*}} ], [ [[INDEX_NEXT:%.*]], {{%.*}} ]
+; CHECK-NEXT:    [[VEC_PHI:%.*]] = phi <vscale x 8 x i32> [ insertelement (<vscale x 8 x i32> zeroinitializer, i32 255, i32 0), {{%.*}} ], [ [[TMP34:%.*]], {{%.*}} ]
+; CHECK-NEXT:    [[VEC_PHI1:%.*]] = phi <vscale x 8 x i32> [ zeroinitializer, {{%.*}} ], [ [[TMP36:%.*]], {{%.*}} ]
----------------
nit: Maybe here and in the PHI below it's good to show where TMP34 and TMP36 come from too, i.e. instead of `{{%.*}}` you can just write `vector.body`.


================
Comment at: llvm/test/Transforms/LoopVectorize/sve-reduction-inloop.ll:14
+; CHECK-NEXT:    [[TMP15:%.*]] = and <vscale x 8 x i32> [[VEC_PHI1]], shufflevector (<vscale x 8 x i32> insertelement (<vscale x 8 x i32> poison, i32 255, i32 0), <vscale x 8 x i32> poison, <vscale x 8 x i32> zeroinitializer)
+; CHECK:         [[TMP21:%.*]] = bitcast i8* {{%.*}} to <vscale x 8 x i8>*
+; CHECK-NEXT:    [[WIDE_LOAD:%.*]] = load <vscale x 8 x i8>, <vscale x 8 x i8>* [[TMP21]], align 4
----------------
nit: It's up to you, but if you prefer a smaller set of CHECK lines you can probably kill off lines TMP21 to TMP25 and just have simple CHECK lines for the loads, i.e.

  ; CHECK:    [[WIDE_LOAD:%.*]] = load <vscale x 8 x i8>, <vscale x 8 x i8>*

and lower down

  ; CHECK;  [[WIDE_LOAD2:%.*]] = load <vscale x 8 x i8>, <vscale x 8 x i8>*




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101260