[PowerPC] add store (load float*) pattern to isProfitableToHoist

[shchenz]

Summary

store (load float*) can be optimized to store(load i32*) in InstCombine pass.

Add store (load float*) to isProfitableToHoist to make sure we don't break the opt in InstCombine pass.

[shchenz]

gentle ping

[shchenz]

gentle ping...

[jsji]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

15997

Looks like the FIXME is wrong? The default is true, so we should be adding more patterns which are NOT profitable to hoist instead?

16027

!I->hasOneUse() is common for all, can we move it up before switch?
Also the Type check can be done after Store opcode check?

16030

If these conditions are *copied* from combineLoadToOperationType, please also copy comments.
Otherwise please add comments about why we need these condition, especially isSwiftError?

Also do we have any IR test with SwiftError?

16033

This is also common, can be moved before switch.

llvm/test/Transforms/SimplifyCFG/PowerPC/prefer-load-i32.ll

1

Precommit the new testcase first so that we can see the diff here.

[shchenz]

address @jsji comments:
1: code refactor
2: split out NFC patch for new added testcase

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

16030

SwiftError attribute can only be applied to pointer to pointer type, so I->getType()->getTypeID() != Type::FloatTyID should get rid of this case.

[jsji]

LGTM.

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;
-
+  switch (I->getOpcode()) {
+  case Instruction::FMul: {
+    // Don't break FMA, PowerPC prefers FMA.
     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));
   }
+  case Instruction::Load: {
+    // Don't break "store (load float*)" pattern, this pattern will be combined
+    // to "store (load int32)" in later InstCombine pass. See function
+    // combineLoadToOperationType. On PowerPC, loading a float point takes more
+    // cycles than loading a 32 bit integer.
+    if (!I->hasOneUse() || I->getType()->getTypeID() != Type::FloatTyID)
+      return true;
+    LoadInst *LI = cast<LoadInst>(I);
+    if (!LI->isUnordered() || LI->getPointerOperand()->isSwiftError())
+      return true;
+
+    Instruction *User = I->user_back();
+    assert(User && "A single use instruction with no uses.");
+
+    if (User->getOpcode() != Instruction::Store)
+      return true;
+
+    return false;
+  }
+  default:
+    return true;
+  }
+  return true;
+}
 
 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[] = {

llvm/test/Transforms/SimplifyCFG/PowerPC/prefer-load-i32.ll

+; RUN: opt < %s -mtriple=powerpc64le-unknown-linux-gnu -simplifycfg -S | \
+; RUN: FileCheck %s
+
+define float @foo(float* %src, float* %dest, i32 signext %count, i32 signext %cond) { 
+entry:
+  %cmp = icmp sgt i32 %cond, 10
+  %idxprom = sext i32 %count to i64
+  %arrayidx = getelementptr inbounds float, float* %src, i64 %idxprom
+  br i1 %cmp, label %if.then, label %if.else
+
+; CHECK-LABEL: if.then:
+; CHECK:  %0 = load float, float* %arrayidx, align 4
+if.then:                                          ; preds = %entry
+  %0 = load float, float* %arrayidx, align 4
+  br label %if.end
+
+; CHECK-LABEL: if.else:
+; CHECK:   %1 = load float, float* %arrayidx, align 4
+; CHECK:   store float %1, float* %arrayidx4, align 4
+if.else:                                          ; preds = %entry
+  %1 = load float, float* %arrayidx, align 4
+  %idxprom3 = sext i32 %count to i64
+  %arrayidx4 = getelementptr inbounds float, float* %dest, i64 %idxprom3
+  store float %1, float* %arrayidx4, align 4
+  br label %if.end
+
+if.end:                                           ; preds = %if.else, %if.then
+  %res2.0 = phi float [ %0, %if.then ], [ 0.000000e+00, %if.else ]
+  ret float %res2.0
+}