[PATCH] D41863: [AArch64] Fix incorrect LD1 of 16-bit FP vectors in big endian

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.td:5852
 def : Pat<(v4f16 (bitconvert (v4i16 FPR64:$src))),
-                             (v4f16 (REV64v4i16 FPR64:$src))>;
+                             (v4f16 FPR64:$src)>;
 def : Pat<(v4f16 (bitconvert (v8i8  FPR64:$src))),
----------------
pbarrio wrote:
> SjoerdMeijer wrote:
> > Don't think I understand why this is now a special case. Why is this one different from the other patterns here?
> Earlier in the file there is an explanation about why we need to insert REVs when we do bitcasts in big endian (~line 5600). At the end, the following comment suggests that identity conversions (e.g. same-size float-to-int conversions) do not need it:
> 
> ```
> 
>  // Most bitconverts require some sort of conversion. The only exceptions are:
>  //   a) Identity conversions -  vNfX <-> vNiX
> ```
> 
> It makes sense to me that a type conversion in big endian between vectors with elements of the same size does not need a byte reversal. This reversal should have been done right before, otherwise the original vector (in this case, with type v4f16) would have also been incorrect.
There should be a testcase specifically for this change, then.  Maybe something like this, to force the conversion:

```
%x = add <4 x i16> %i, 1
%y = bitcast <4 x i16> %x to <4 x half>
%z = fpext <4 x half> %y tp <4 x float>
```

Please put this in a separate patch from the AArch64TargetLowering::addTypeForNEON changes.  Also, please move the pattern out of the IsBE predicate.


https://reviews.llvm.org/D41863