[PowerPC] Don't convert Loop to CTR Loop for fp128 BinaryOperator

[ZhangKang]

Summary

If the instruction in the Loop use CTR, we won't convert loop to CTR loop.
For PPC BinaryOperator of fp128 will become libcall, we shouldn't convert loop to
CTR loop if the loop contain libCall.

But currently, in the PPCTTIImpl::mightUseCTR() function, we only deal with
BinaryOperator for ppc_fp128, don't deal with the fp128.

This bug has caused assertion error, when the loop we want to convert it to CTL loop
contains BinaryOperator for ppc_fp128 like %mul = fmul fp128 %0, %1.

[efriedma]

Not a comment on this patch specifically, but in general, I'd recommend changing the PowerPC implementation of hardware loops to switch to an approach that allows making the final decision on whether to emit a hardware loop after isel. ARM does this.

Basically, if that target sets HWLoopInfo.CounterInReg to true in isHardwareLoopProfitable, the HardwareLoops pass emits slightly different intrinsics; the alternate intrinsics are more straightforward to lower to a regular compare/branch. Then, if the target decides after isel that a hardware loop is illegal or unprofitable, it can easily fall back. That way, we can avoid this endless stream of bugs: if isHardwareLoopProfitable misses some obscure construct that requires clobbering CTR, it doesn't matter.

[steven.zhang]

In D81353#2078725, @efriedma wrote:

Not a comment on this patch specifically, but in general, I'd recommend changing the PowerPC implementation of hardware loops to switch to an approach that allows making the final decision on whether to emit a hardware loop after isel. ARM does this.

Basically, if that target sets HWLoopInfo.CounterInReg to true in isHardwareLoopProfitable, the HardwareLoops pass emits slightly different intrinsics; the alternate intrinsics are more straightforward to lower to a regular compare/branch. Then, if the target decides after isel that a hardware loop is illegal or unprofitable, it can easily fall back. That way, we can avoid this endless stream of bugs: if isHardwareLoopProfitable misses some obscure construct that requires clobbering CTR, it doesn't matter.

Cannot agree with this suggestion any more. We are struggling to sync the logic with isel that is hard to maintain.

[shchenz]

I think it is a good idea to decide hardware loop or not after ISEL where IR operations with a certain type will be lowered.
However, this improvement should be aside from this bug fix patch.

This patch LGTM

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

if (CallInst *CI = dyn_cast<CallInst>(J)) {
             continue;
 
           return true;
         }
       }
 
       return true;
     } else if (isa<BinaryOperator>(J) &&
-               J->getType()->getScalarType()->isPPC_FP128Ty()) {
-      // Most operations on ppc_f128 values become calls.
+               (J->getType()->getScalarType()->isFP128Ty() ||
+                J->getType()->getScalarType()->isPPC_FP128Ty())) {
+      // Most operations on f128 or ppc_f128 values become calls.
       return true;
     } else if (isa<UIToFPInst>(J) || isa<SIToFPInst>(J) ||
                isa<FPToUIInst>(J) || isa<FPToSIInst>(J)) {
       CastInst *CI = cast<CastInst>(J);
       if (CI->getSrcTy()->getScalarType()->isPPC_FP128Ty() ||
           CI->getDestTy()->getScalarType()->isPPC_FP128Ty() ||
           isLargeIntegerTy(!TM.isPPC64(), CI->getSrcTy()->getScalarType()) ||
           isLargeIntegerTy(!TM.isPPC64(), CI->getDestTy()->getScalarType()))

llvm/test/CodeGen/PowerPC/ctrloop-fp128.ll

for.body:                                         ; preds = %for.body, %entry
   %inc = add nuw nsw i64 %i.06, 1
   %exitcond = icmp eq i64 %inc, 4
   br i1 %exitcond, label %for.end, label %for.body
 
 for.end:                                          ; preds = %for.body
   ret void
 
 ; CHECK-LABEL: fmul_ctrloop_fp128
-; CHECK:         call void @llvm.set.loop.iterations.i64(i64 4)
-; CHECK:         call i1 @llvm.loop.decrement.i64(i64 1)
+; CHECK-NOT:     call void @llvm.set.loop.iterations.i64(i64 4)
+; CHECK-NOT:     call i1 @llvm.loop.decrement.i64(i64 1)
 }
 
 declare void @obfuscate(i8*, ...) local_unnamed_addr #2