[PATCH] D78668: PowerPC: Treat llvm.fma.f* intrinsic as using CTR with SPE

[Justin Hibbits via Phabricator via llvm-commits]

jhibbits marked 3 inline comments as done.
jhibbits added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp:320
+            if (ST->hasSPE())
+              return true; // ISD::FMA might be a libcall on SPE
+            else
----------------
shchenz wrote:
> In `PPCTargetLowering::PPCTargetLowering`, We already have 
> ```
>  if (Subtarget.hasSPE()) {
>     setOperationAction(ISD::FMA  , MVT::f64, Expand);
>     setOperationAction(ISD::FMA  , MVT::f32, Expand);
>  }
> ```
> 
> And there is a hook function `TLI->isOperationLegalOrCustom(Opcode, EVTy)` at line 415. I think better to use that function to check if one operation with a type is legal or not, similar with following `Intrinsic::sqrt`?
I'm surprised I missed that, quite the oversight.  Yes, that test below is much preferred, I was afraid I might need to play whack-a-mole on this, and wondering why that wasn't the case except for FMA.  Fixing it to the better way.


================
Comment at: llvm/test/CodeGen/PowerPC/spe.ll:1405
 ; CHECK-NEXT:    blr
-entry:
   %b = getelementptr %struct.a, %struct.a* %f, i32 0, i32 0
----------------
shchenz wrote:
> Accidentally delete?
Yeah.  a botched rebase sorting my local commits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78668