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

[Peter Waller via Phabricator via llvm-commits]

peterwaller-arm marked an inline comment as done.
peterwaller-arm added a comment.

I'm going to abandon this for now, though I may take another swipe at this later. I've identified while testing other types that this patch is wrong because it's not returning VectorRC when it should.



================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:229
+    return Vector ? 1 : 0;
+  return getRegMVTForType(Ty).isVector() ? 1 : 0;
+}
----------------
peterwaller-arm wrote:
> 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.
> Can this just use the existing TLI->getTypeLegalizationCost call to get the MVT?

It can however directly use `getTLI()->getRegisterType`, so I'm thinking I shouldn't introduce another TTI. I was however thinking along the lines of getRegUsage returning this information alongside the register count, hence putting it nearby.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125956