[PATCH] D90233: [AArch64] Use DUP for BUILD_VECTOR with few different elements.

Wed Oct 28 08:21:59 PDT 2020

fhahn marked an inline comment as done.
fhahn added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:9205
+      Ops.push_back(Value);
+    SDValue NewVector = LowerBUILD_VECTOR(DAG.getBuildVector(VT, dl, Ops), DAG);
+    // Next, insert the elements that do not match the common value.
----------------
dmgreen wrote:
> Could this just create a DUP? Or is this so that it does not have to figure out what kind of DUP/DUPLANE to use?
We could, but I am not sure how much extra logic would be required to handle constant and non-constant splats and other cases optimized by LowerBUILD_VECTOR. This seems to also be in-line  with other parts of this function that also delegate back to LowerBUILD_VECTOR.

================
Comment at: llvm/test/CodeGen/AArch64/arm64-vector-insertion.ll:33

 ; TODO: This should jsut be a dup + clearing lane 4.
 define <4 x float> @test_insert_3_f32_undef_zero_vector(float %a) {
----------------
dmgreen wrote:
> Comment can be updated too
yes, removed!

================
Comment at: llvm/test/CodeGen/AArch64/arm64-vector-insertion.ll:41
+; CHECK-NEXT:   mov.s v0[3], v1[0]
 ; CHECK-NEXT:   ret
 ;
----------------
efriedma wrote:
> The test coverage here seems a bit lacking.
I added more positive and negative tests in 5febc535c4f8.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90233