[PATCH] D80485: [DAGCombiner][PowerPC][AArch64] Remove isMulhCheaperThanMulShift TLI hook. Use isOperationLegal directly instead.

[Amy Kwan via Phabricator via llvm-commits]

amyk added a comment.

So, my goal was to introduce a DAG combine to combine multiply+shifts into mulh. This is done in a function I introduced within `DAGCombiner.cpp` (called `combineShiftToMULH`).

Since I was implementing something that is in target independent code, I thought it may be better to enable it on PowerPC only first since I was not quite sure if other targets were interested it this and I was seeing many other LIT failures at the time. 
Thus, I introduced the hook (`isMulhCheaperThanMulShift`) and targets who wish to combine multiply+shifts into mulh could implement that hook. Within `combineShiftToMULH`, there is a check to `isMulhCheaperThanMulShift`.

If we wanted to enable this for everyone, I think the check `isOperationLegalOrCustom` would probably be sufficient. I just tried this patch again, and made the following changes for `combineShiftToMULH`:

  @@ -8099,12 +8101,6 @@ static SDValue combineShiftToMULH(SDNode *N, SelectionDAG &DAG,
     if (NarrowVT !=  RightOp.getOperand(0).getValueType())
       return SDValue();
  
  -  // Only transform into mulh if mulh for the narrow type is cheaper than
  -  // a multiply followed by a shift. This should also check if mulh is
  -  // legal for NarrowVT on the target.
  -  if (!TLI.isMulhCheaperThanMulShift(NarrowVT))
  -      return SDValue();
  -
     // Proceed with the transformation if the wide type is twice as large
     // as the narrow type.
     unsigned NarrowVTSize = NarrowVT.getScalarSizeInBits();
  @@ -8122,6 +8118,12 @@ static SDValue combineShiftToMULH(SDNode *N, SelectionDAG &DAG,
     // we use mulhs. Othewise, zero extends (zext) use mulhu.
     unsigned MulhOpcode = IsSignExt ? ISD::MULHS : ISD::MULHU;
  
  +  // Only transform into mulh if mulh for the narrow type is cheaper than
  +  // a multiply followed by a shift. This should also check if mulh is
  +  // legal for NarrowVT on the target.
  +  if (!TLI.isOperationLegalOrCustom(MulhOpcode, NarrowVT))
  +      return SDValue();
  +
     SDValue Result = DAG.getNode(MulhOpcode, DL, NarrowVT, LeftOp.getOperand(0),
                                  RightOp.getOperand(0));
     return (N->getOpcode() == ISD::SRA ? DAG.getSExtOrTrunc(Result, DL, WideVT1)

I only see one LIT failure now:

  Failed Tests (1):
    LLVM :: CodeGen/X86/pmulh.ll


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

https://reviews.llvm.org/D80485