[PATCH] D41863: [AArch64] Fix incorrect LD1 of 16-bit FP vectors in big endian
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 15 12:52:38 PST 2018
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
More information about the llvm-commits
mailing list