[PATCH] D94073: [RISCV] Add vector integer mul/mulh/div/rem ISel patterns

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D94073#2478965 <https://reviews.llvm.org/D94073#2478965>, @frasercrmck wrote:

> For a bit more context around the (surprising) `createConstant` issue:
>
>   // In other cases the element type is illegal and needs to be expanded, for
>   // example v2i64 on MIPS32. In this case, find the nearest legal type, split
>   // the value into n parts and use a vector type with n-times the elements.
>   // Then bitcast to the type requested.
>   // Legalizing constants too early makes the DAGCombiner's job harder so we
>   // only legalize if the DAG tells us we must produce legal types.
>   else if (NewNodesMustHaveLegalTypes && VT.isVector() &&
>            TLI->getTypeAction(*getContext(), EltVT) ==
>            TargetLowering::TypeExpandInteger) {
>     const APInt &NewVal = Elt->getValue();
>     EVT ViaEltVT = TLI->getTypeToTransformTo(*getContext(), EltVT);
>     unsigned ViaEltSizeInBits = ViaEltVT.getSizeInBits();
>     unsigned ViaVecNumElts = VT.getSizeInBits() / ViaEltSizeInBits;
>     EVT ViaVecVT = EVT::getVectorVT(*getContext(), ViaEltVT, ViaVecNumElts);
>   
>     // Check the temporary vector is the correct size. If this fails then
>     // getTypeToTransformTo() probably returned a type whose size (in bits)
>     // isn't a power-of-2 factor of the requested type size.
>     assert(ViaVecVT.getSizeInBits() == VT.getSizeInBits());
>
> It should be obvious to see how this has no chance of working for scalable vectors.
>
> I don't fully understand the constraints on `NewNodesMustHaveLegalTypes`: is it permissible to ask the target to custom-lower this case if they've marked `SPLAT_VECTOR` as `Custom`? Obviously we'd like this to be lowered to our `SPLAT_VECTOR_I64` if possible and to our custom sequence otherwise.

I'm not sure if we can call the custom handlers from inside of createConstant. We're technically in DAG combine not a legalization step. Why is DAGCombiner calling createConstant?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94073