[PowerPC] implement target hook isProfitableToHoist

[shchenz]

Summary

On Powerpc fma is faster than fadd + fmul for some types, (PPCTargetLowering::isFMAFasterThanFMulAndFAdd). we should implement target hook isProfitableToHoist to prevent simplifyCFGpass from breaking fma pattern by hoisting fmul to predecessor block.

[nemanjai]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

15390

Can we please not have the same logic in two places. Please either call this one from the other by using VT.getTypeForEVT() and MF.getFunction()
or write a separate helper function that can be called by both.

llvm/test/Transforms/SimplifyCFG/PowerPC/prefer-fma.ll

4

I think it would be preferable to pre-commit this test case with full checks produced by the script and then just show the differences here.

[shchenz]

split out nfc patch and testcase baseline

[shchenz]

Thanks for your quick review @nemanjai

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

15390

Split out this function as NFC patch in D76265.

[nemanjai]

LGTM aside from a nit from me and a couple from clang-format. I'm sure you can address those on the commit.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

15399

I think we can omit the check for nullptr here since you have already checked the instruction has a use. Perhaps just change it to:
assert(User && "A single use instruction with no uses?");

Also, I think it is more common to De-Morgan-ize these conditions to something like

if (User->getOpcode() != Instruction::FSub &&
    User->getOpcode() != Instruction::FAdd)

[shchenz]

address comments

llvm/lib/Target/PowerPC/PPCISelLowering.h

public:
     /// than a pair of fmul and fadd instructions. fmuladd intrinsics will be
     /// expanded to FMAs when this method returns true, otherwise fmuladd is
     /// expanded to fmul + fadd.
     bool isFMAFasterThanFMulAndFAdd(const MachineFunction &MF,
                                     EVT VT) const override;
 
     bool isFMAFasterThanFMulAndFAdd(const Function &F, Type *Ty) const override;
 
+    /// isProfitableToHoist - Check if it is profitable to hoist instruction
+    /// \p I to its dominator block.
+    /// For example, it is not profitable if \p I and it's only user can form a
+    /// FMA instruction, because Powerpc prefers FMADD.
+    bool isProfitableToHoist(Instruction *I) const override;
+
     const MCPhysReg *getScratchRegisters(CallingConv::ID CC) const override;
 
     // Should we expand the build vector with shuffles?
     bool
     shouldExpandBuildVectorWithShuffles(EVT VT,
                                         unsigned DefinedValues) const override;
 
     /// createFastISel - This method returns a target-specific FastISel object,

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

case Type::DoubleTyID:
     return true;
   case Type::FP128TyID:
     return EnableQuadPrecision && Subtarget.hasP9Vector();
   default:
     return false;
   }
 }
 
+// Currently this is a copy from AArch64TargetLowering::isProfitableToHoist.
+// FIXME: add more patterns which are profitable to hoist.
+bool PPCTargetLowering::isProfitableToHoist(Instruction *I) const {
+  if (I->getOpcode() != Instruction::FMul)
+    return true;
+
+  if (!I->hasOneUse())
+    return true;
+
+  Instruction *User = I->user_back();
+  assert(User && "A single use instruction with no uses.");
+
+  if (User->getOpcode() != Instruction::FSub &&
+      User->getOpcode() == Instruction::FAdd)
+    return true;
+
+  const TargetOptions &Options = getTargetMachine().Options;
+  const Function *F = I->getFunction();
+  const DataLayout &DL = F->getParent()->getDataLayout();
+  Type *Ty = User->getOperand(0)->getType();
+
+  return !(
+      isFMAFasterThanFMulAndFAdd(*F, Ty) &&
+      isOperationLegalOrCustom(ISD::FMA, getValueType(DL, Ty)) &&
+      (Options.AllowFPOpFusion == FPOpFusion::Fast || Options.UnsafeFPMath));
+}
+
 const MCPhysReg *
 PPCTargetLowering::getScratchRegisters(CallingConv::ID) const {
   // LR is a callee-save register, but we must treat it as clobbered by any call
   // site. Hence we include LR in the scratch registers, which are in turn added
   // as implicit-defs for stackmaps and patchpoints. The same reasoning applies
   // to CTR, which is used by any indirect call.
   static const MCPhysReg ScratchRegs[] = {
     PPC::X12, PPC::LR8, PPC::CTR8, 0

llvm/test/Transforms/SimplifyCFG/PowerPC/prefer-fma.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -mtriple=powerpc64le-unknown-linux-gnu -simplifycfg -enable-unsafe-fp-math -S | \
 ; RUN: FileCheck %s
 
 ; This case is copied from test/Transforms/SimplifyCFG/AArch64/
 ; Function Attrs: nounwind
 define double @_Z3fooRdS_S_S_(double* dereferenceable(8) %x, double* dereferenceable(8) %y, double* dereferenceable(8) %a) {
 ; CHECK-LABEL: @_Z3fooRdS_S_S_(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = load double, double* [[Y:%.*]], align 8
 ; CHECK-NEXT:    [[CMP:%.*]] = fcmp oeq double [[TMP0]], 0.000000e+00
 ; CHECK-NEXT:    [[TMP1:%.*]] = load double, double* [[X:%.*]], align 8
 ; CHECK-NEXT:    [[TMP2:%.*]] = load double, double* [[A:%.*]], align 8
-; CHECK-NEXT:    [[TMP3:%.*]] = fmul fast double [[TMP1]], [[TMP2]]
 ; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
 ; CHECK:       if.then:
+; CHECK-NEXT:    [[TMP3:%.*]] = fmul fast double [[TMP1]], [[TMP2]]
 ; CHECK-NEXT:    [[MUL:%.*]] = fadd fast double 1.000000e+00, [[TMP3]]
 ; CHECK-NEXT:    store double [[MUL]], double* [[Y]], align 8
 ; CHECK-NEXT:    br label [[IF_END:%.*]]
 ; CHECK:       if.else:
-; CHECK-NEXT:    [[SUB1:%.*]] = fsub fast double [[TMP3]], [[TMP0]]
+; CHECK-NEXT:    [[MUL1:%.*]] = fmul fast double [[TMP1]], [[TMP2]]
+; CHECK-NEXT:    [[SUB1:%.*]] = fsub fast double [[MUL1]], [[TMP0]]
 ; CHECK-NEXT:    [[GEP1:%.*]] = getelementptr double, double* [[Y]], i32 1
 ; CHECK-NEXT:    store double [[SUB1]], double* [[GEP1]], align 8
 ; CHECK-NEXT:    br label [[IF_END]]
 ; CHECK:       if.end:
 ; CHECK-NEXT:    [[TMP4:%.*]] = load double, double* [[Y]], align 8
 ; CHECK-NEXT:    [[CMP2:%.*]] = fcmp oeq double [[TMP4]], 2.000000e+00
 ; CHECK-NEXT:    [[TMP5:%.*]] = load double, double* [[X]], align 8
 ; CHECK-NEXT:    br i1 [[CMP2]], label [[IF_THEN2:%.*]], label [[IF_ELSE2:%.*]]