[PATCH] D152205: [Aarch64][SVE]SVE2] Enable tbl, tbl2 for shuffle lowering for fixed vector types.

[Dinar Temirbulatov via Phabricator via llvm-commits]

dtemirbulatov marked 2 inline comments as done.
dtemirbulatov added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:25761
+      Offset = 255;
+    else if (Offset >= NumElts && FillElements)
+      Offset += IndexLen - NumElts;
----------------
CarolineConcatto wrote:
> For me this tests is a bit odd.
> FillElements is unsigned and not a bool. So are you checking if IndexLen> NumElts?
> Is it too difficult to put and example for each case with the fixed mask. So we can see how the mask was and how it has changed.
>  if (Swap)
>  for Offset < NumElts ->mask = <0,0,7,7> -> <3,3,0,0>
>  for Offset > NumElts ->mask = <0,0,7,7>
> 
> Maybe this will help to understand if there is any hidden problem with these ifs.
> 
> I re-wrote that to something like:
> 
> ```
>   // Set the Mask with the correct indexes
>   SmallVector<SDValue, 8> TBLMask;
>   for (int Val : ShuffleMask) {
>     unsigned Offset = Val;
>     if (Swap)
>       Offset = Offset < NumElts ? Offset + NumElts : Offset - NumElts;
>     // why we need to set the mask to 255 when one of the vectors is zero/undef?
>     else if(Offset >= NumElts){
>        if(IsUndefOrZero)
>            Offset = 255;
>        else if (IndexLen > NumElts)
>            Offset += IndexLen - NumElts;
>     }
>     TBLMask.push_back(DAG.getConstant(Offset, DL, MVT::i64));
>   }
> ```
Thanks for finding this, I think now that "IndexLen> NumElts" logic/transform was not correct.


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

https://reviews.llvm.org/D152205