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

[Qiu Chaofan via Phabricator via llvm-commits]

qiucf added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp:663
+               J->getOperand(0)->getType()->getScalarType()->isFP128Ty()) {
+      return true;
     } else if (isa<UIToFPInst>(J) || isa<SIToFPInst>(J) ||
----------------
shchenz wrote:
> 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?
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)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117459