[PowerPC] Change CTR clobber estimation for 128-bit floating types

[qiucf]

Summary

Restrict fpext and fptrunc cast instructions to/from fp128.

We need a more complete fix to defer CTR clobber judgement after instruction selection. See D81353.

[qiucf]

Fix check for fcmp because its result type is not float

[shchenz]

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

663

I think we are trying to handle fpext/fptrunc for fp128 type? If so, can we just explicitly handle them like:

} else if (isa<FPTruncInst>(J) && cast<FPTruncInst>(J)->getSrcTy()->getScalarType()->isFP128Ty()) {
  return true;
} else  if (isa<FPExtInst>(J) && cast<FPExtInst>(J)->getDestTy()->getScalarType()->isFP128Ty()) {
  return true;
}

Your patch seems not only change instructions FPTruncInst and FPExtInst. As you may have noted, ctr clobber check here is very sensitive especially for fp128/ppc_fp128. We met several issues before.

And have you checked with ppc_fp128 for the above two instructions FPTruncInst and FPExtInst? Can we expand them on PowerPC without potential writing ctr?

[qiucf]

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

663

Yes. Because of the structure of ppc_fp128:

• fptrunc ppc_fp128 %a to double: nothing (just return first fpr)
• fptrunc ppc_fp128 %a to float: xsrsp (trunc the first fpr to float)
• fpext double %a to ppc_fp128: xxlxor (set the second part as zero)
• fpext float %a to ppc_fp128: xxlxor (set the second part as zero)

[shchenz]

Thanks for fixing this. LGTM

[nemanjai]

I think that since we still keep getting issues with this after so many years and so many changes, we should just reject any operations on IBM double-double and fp128 until we refactor the code as suggested previously.

[qiucf]

Restrict all operations with ppc_fp128/fp128 result type

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

if (CallInst *CI = dyn_cast<CallInst>(J)) {
                    TLI->isOperationLegalOrCustom(Opcode, EVTy.getScalarType()))
             continue;
 
           return true;
         }
       }
 
       return true;
-    } else if (isa<BinaryOperator>(J) &&
-               (J->getType()->getScalarType()->isFP128Ty() ||
+    } else if ((J->getType()->getScalarType()->isFP128Ty() ||
                 J->getType()->getScalarType()->isPPC_FP128Ty())) {
       // Most operations on f128 or ppc_f128 values become calls.
       return true;
+    } else if (isa<FCmpInst>(J) &&
+               J->getOperand(0)->getType()->getScalarType()->isFP128Ty()) {
+      return true;
+    } else if ((isa<FPTruncInst>(J) || isa<FPExtInst>(J)) &&
+               (cast<CastInst>(J)->getSrcTy()->getScalarType()->isFP128Ty() ||
+                cast<CastInst>(J)->getDestTy()->getScalarType()->isFP128Ty())) {
+      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()))
         return true;

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

-; RUN: llc -verify-machineinstrs -stop-after=hardware-loops -mcpu=pwr9 \
-; RUN:   -mtriple=powerpc64le-unknown-unknown < %s | FileCheck %s
+; RUN: llc < %s -verify-machineinstrs -stop-after=hardware-loops -mcpu=pwr9 \
+; RUN:   -mtriple=powerpc64le-unknown-unknown | FileCheck %s
+; RUN: llc < %s -verify-machineinstrs -stop-after=hardware-loops -mcpu=pwr8 \
+; RUN:   -mtriple=powerpc64le-unknown-unknown | FileCheck %s
 
 @a = internal global fp128 0xL00000000000000000000000000000000, align 16
 @x = internal global [4 x fp128] zeroinitializer, align 16
 @y = internal global [4 x fp128] zeroinitializer, align 16
 
 define void @fmul_ctrloop_fp128() {
 entry:
   %0 = load fp128, fp128* @a, align 16

 for.end:                                          ; preds = %for.body
   ret void
 
 ; CHECK-LABEL: fmul_ctrloop_fp128
 ; CHECK-NOT:     call void @llvm.set.loop.iterations.i64(i64 4)
 ; CHECK-NOT:     call i1 @llvm.loop.decrement.i64(i64 1)
 }
 
+define void @fpext_ctrloop_fp128(double* %a) {
+entry:
+  br label %for.body
+
+for.body:
+  %i.06 = phi i64 [ 0, %entry ], [ %inc, %for.body ]
+  %arrayidx = getelementptr inbounds double, double* %a, i64 %i.06
+  %0 = load double, double* %arrayidx, align 8
+  %ext = fpext double %0 to fp128
+  %arrayidx1 = getelementptr inbounds [4 x fp128], [4 x fp128]* @y, i64 0, i64 %i.06
+  store fp128 %ext, fp128* %arrayidx1, align 16
+  %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:
+  ret void
+
+; CHECK-LABEL: fpext_ctrloop_fp128
+; CHECK-NOT: call void @llvm.set.loop.iterations.i64(i64 4)
+; CHECK-NOT: call i1 @llvm.loop.decrement.i64(i64 1)
+}
+
+define void @fptrunc_ctrloop_fp128(double* %a) {
+entry:
+  br label %for.body
+
+for.body:
+  %i.06 = phi i64 [ 0, %entry ], [ %inc, %for.body ]
+  %arrayidx = getelementptr inbounds [4 x fp128], [4 x fp128]* @x, i64 0, i64 %i.06
+  %0 = load fp128, fp128* %arrayidx, align 16
+  %trunc = fptrunc fp128 %0 to double
+  %arrayidx1 = getelementptr inbounds double, double* %a, i64 %i.06
+  store double %trunc, double* %arrayidx1, align 16
+  %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:
+  ret void
+
+; CHECK-LABEL: fptrunc_ctrloop_fp128
+; 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