[PATCH] D86114: [SVE] Lower fixed length vXi8/vXi16 SDIV to scalable

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

In D86114#2227054 <https://reviews.llvm.org/D86114#2227054>, @cameron.mcinally wrote:

> In D86114#2226955 <https://reviews.llvm.org/D86114#2226955>, @efriedma wrote:
>
>> The end result is a big uglier than I would have hoped for, but I can't think of any particularly better way given the constraints.
>
> Agreed. I'll wait for Paul to have a look to see if he has any tricks.

I don't think there's a very different approach because you really want custom type legalisation for a legal type, which would be nice but doesn't exists.  That said the custom lowering for i8/i16 doesn't have to be SVE specific as there's the alternative approach of using normal ISD nodes to do the widening so that only the final SDIV lowering is SVE specific.  I'm thinking of the following:

  std::tie(Op0Lo, Op0Hi) = DAG.SplitVector(Op.getOperand(0), DL);
  std::tie(Op1Lo, Op1Hi) = DAG.SplitVector(Op.getOperand(1), DL);
  
  Op0Lo = DAG.getNode(ISD::ZERO_EXTEND, DL, FixedWidenedVT, Op0Lo);
  Op1Lo = DAG.getNode(ISD::ZERO_EXTEND, DL, FixedWidenedVT, Op1Lo);
  Op0Hi = DAG.getNode(ISD::ZERO_EXTEND, DL, FixedWidenedVT, Op0Hi);
  Op1Hi = DAG.getNode(ISD::ZERO_EXTEND, DL, FixedWidenedVT, Op1Hi);
  
  ResultLo = DAG.getNode(Op.getOpcode(), DL, FixedWidenedVT, Op0Lo, Op0Hi);
  ResultHi = DAG.getNode(Op.getOpcode(), DL, FixedWidenedVT, Op0Lo, Op0Hi);
  
  ResultLo = DAG.getNode(ISD::TRUNCATE, DL, HalfVT, ResultLo);
  ResultHi = DAG.getNode(ISD::TRUNCATE, DL, HalfVT, ResultHi);
  
  return DAG.getNode(ISD::CONCAT_VECTORS, DL, VT, ResultLo, ResultHi);

Which I think looks nicer and might automatically benefit from optimising the "split" away when the resulting expanded operands can fit within a single register.  The big downside to this is that bigger than NEON fixed length EXTRACT_SUBVECTOR and CONCAT_VECTORS support doesn't exist so it'll either not work today or result in terrible code.

Based on this I'm happy to take the above as a refactoring exercise (which I'm happy to do if you want) and have the patch landed in its current form.


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

https://reviews.llvm.org/D86114