[PATCH] D125956: [NOT YET FOR REVIEW][AArch64][LV] Implement AArch64TTIImpl::getRegisterClassForType

[Peter Waller via Phabricator via llvm-commits]

peterwaller-arm marked 2 inline comments as done.
peterwaller-arm added a comment.

On the effect of this patch: there are 10 translation units out of ~2,000 in the LNT benchmarks which have differing codegen as a consequence. I'll check for any performance effect next week before proposing this.

  MultiSource/Applications/JM/ldecod/CMakeFiles/ldecod.dir/erc_do_p.o
  MultiSource/Applications/JM/lencod/CMakeFiles/lencod.dir/me_distortion.o
  MultiSource/Benchmarks/mediabench/mpeg2/mpeg2dec/CMakeFiles/mpeg2decode.dir/recon.o
  MultiSource/Benchmarks/mediabench/jpeg/jpeg-6a/CMakeFiles/cjpeg.dir/jquant2.o
  MultiSource/Benchmarks/Bullet/CMakeFiles/bullet.dir/btQuantizedBvh.cpp.opt.yaml
  MultiSource/Benchmarks/MiBench/automotive-susan/CMakeFiles/automotive-susan.dir/susan.o
  MultiSource/Benchmarks/MiBench/consumer-jpeg/CMakeFiles/consumer-jpeg.dir/jquant2.o
  MultiSource/Benchmarks/7zip/CMakeFiles/7zip-benchmark.dir/CPP/7zip/Crypto/WzAes.cpp.opt.yaml
  MultiSource/Benchmarks/7zip/CMakeFiles/7zip-benchmark.dir/C/Delta.o
  SingleSource/Benchmarks/Misc/CMakeFiles/mandel.dir/mandel.o



================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:229
+    return Vector ? 1 : 0;
+  return getRegMVTForType(Ty).isVector() ? 1 : 0;
+}
----------------
dmgreen wrote:
> peterwaller-arm wrote:
> > dmgreen wrote:
> > > Can this just use the existing TLI->getTypeLegalizationCost call to get the MVT?
> > As far as I can see, no, because the return value of `getTypeLegalizationCost` is wrong, deriving its value from `getValueType`. This just does `EVT::getVectorVT(Ty->getContext(), EVT::getEVT(EltTy, false),` under the hood, which is different than `TLI->getRegisterType`, which uses information derived from `computeRegisterProperties`.
> I would expect getTypeLegalizationCost(..).second to be a scalar type if the original vector was scalarized. It goes via the legalization mechanism in getTypeConversion.
`getTypeConversion` appears to use `TransformToType`. Here is the information I've dumped from computeRegisterProperties with vscale_range(4,4) (512 bit reg):

```
CRP:     v1i1 PreferredAction=TypeScalarizeVector NumRegisters=1 NumIntermediates=1 VT=    v1i1 IntermediateVT=      i1 RegisterVT=     i32 Final ValueTypeActions[VT]= TypeScalarizeVector TransformToType=    v1i1
CRP:     v2i1 PreferredAction=TypePromoteInteger Promote    v2i32
CRP:     v4i1 PreferredAction=TypePromoteInteger Promote    v4i16
CRP:     v8i1 PreferredAction=TypePromoteInteger Promote     v8i8
CRP:    v16i1 PreferredAction=TypePromoteInteger Promote    v16i8
CRP:    v32i1 PreferredAction=TypePromoteInteger Promote    v32i8
CRP:    v64i1 PreferredAction=TypePromoteInteger Promote    v64i8
CRP:   v128i1 PreferredAction=TypePromoteInteger NumRegisters=128 NumIntermediates=128 VT=  v128i1 IntermediateVT=      i1 RegisterVT=     i32 Final ValueTypeActions[VT]= TypeSplitVector TransformToType=  v128i1
CRP:   v256i1 PreferredAction=TypePromoteInteger NumRegisters=256 NumIntermediates=256 VT=  v256i1 IntermediateVT=      i1 RegisterVT=     i32 Final ValueTypeActions[VT]= TypeSplitVector TransformToType=  v256i1
CRP:   v512i1 PreferredAction=TypePromoteInteger NumRegisters=512 NumIntermediates=512 VT=  v512i1 IntermediateVT=      i1 RegisterVT=     i32 Final ValueTypeActions[VT]= TypeSplitVector TransformToType=  v512i1
CRP:  v1024i1 PreferredAction=TypePromoteInteger NumRegisters=1024 NumIntermediates=1024 VT= v1024i1 IntermediateVT=      i1 RegisterVT=     i32 Final ValueTypeActions[VT]= TypeSplitVector TransformToType= v1024i1
```

For example, getTypeLegalizationCost uses the TransformToType of v128i1, not understanding that it use the RegisterVT of i32.

This could be considered buggy too, I started playing around with this but found myself mired in failing tests, so I chose the easier option as a first pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125956