[PATCH] D84995: [AMDGPU][CostModel] Add f16, f64 and contract cases to fused costs estimation.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp:514
+    if ((SLT == MVT::f32 || SLT == MVT::f64 || SLT == MVT::f16) && CxtI &&
+        CxtI->hasOneUse())
       if (const auto *FAdd = dyn_cast<BinaryOperator>(*CxtI->user_begin())) {
----------------
I think we don't actually need the hasOneUse case. Only with the conditionally available 2-operand VOP2 forms is there a code size savings by not fusing 


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp:518
+        if ((OPC == ISD::FADD || OPC == ISD::FSUB) &&
+            (!HasFP32Denormals ||
+             (CxtI->hasAllowContract() && FAdd->hasAllowContract())))
----------------
The !HasFP32Denormals check is low applying to all types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84995