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

[guopeilin via Phabricator via llvm-commits]

guopeilin added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:9471
+    if (ExtractedValType.isFixedLengthVector() &&
+        ExtractedValType.getSizeInBits().getValue() <= 128) {
+      Lane += V.getConstantOperandVal(1);
----------------
sdesmalen wrote:
> david-arm wrote:
> > nit: Before committing could you change this to:
> > 
> >   ExtractedValType.getFixedSizeInBits()
> > 
> > It's a bit shorter and it also asserts that the TypeSize is fixed.
> It should be fine to extract:
> 
>   v2f32 (extract v16f32 X, 2), 1 -> dup X, 3
> 
> The current code seems to check that sizeof(v16f32) <= 128, whereas it should use the index `ExtractedValType.getVectorElementType().getSizeInBits() * (Lane + V.getConstantOperandVal(1))` to determine whether the lane can be indexed..
> 
> Additionally, the 128 seems conservative as well, since SVE indexed DUP allows an index < 512bits.
> 
Thanks for reviewing.
And there is something that confused me, I just wonder what exactly is the value of `ExtractedValType.getVectorElementType().getSizeInBits() * (Lane + V.getConstantOperandVal(1))` represents for? For your example, 
here I guess the value if `f32 x 3` (Correct me if I am wrong), which represents neither the `v2f32`(return value) nor the `v16f32`(input value). And what should the `f32 x 3` compare with? 


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

https://reviews.llvm.org/D110524