[PATCH] D110524: [AArch64ISelLowering] Avoid duplane in some cases when sve enabled

[David Sherwood via Phabricator via llvm-commits]

david-arm added a comment.

Can you change the name of the existing test from sve-limit-duplane.ll -> sve-fixed-length-limit-duplane.ll? This is the convention we use when using SVE for fixed-length vectors.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:9470
+    auto ExtractedValTye = V.getOperand(0).getValueType();
+    if (ExtractedValTye.getSizeInBits().getValue() <= 128) {
+      Lane += V.getConstantOperandVal(1);
----------------
This isn't safe for truly scalable vectors where the size is unknown at runtime. I think this should probably be written as:

  auto ExtractedValType = V.getOperand(0).getValueType();
  if (ExtractedValType.isFixedLengthVector() && ExtractedValType.getFixedSizeInBits() <= 128) {
   ...
  }


  


================
Comment at: llvm/test/CodeGen/AArch64/sve-limit-duplane.ll:30
+; CHECK-NEXT:    ret
+entry:
+  %0 = load <16 x i32>, <16 x i32>* %arg, align 256
----------------
If possible I think it would be better to try to reduce this test case to the raw operations using proper scalable vectors, i.e. something like:

  define <4 x i32> @test_work_knt_val(<16 x i32> %arg) {
    %shvec = shufflevector <16 x i32> %arg, <16 x i32> undef, <4 x i32> <i32 14, i32 14, i32 14, i32 14>
    ret <4 x i32> %shvec
  }


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

https://reviews.llvm.org/D110524