[PATCH] D154528: [AMDGPU][GlobalISel] Generate fast fp64-to-fp16 conversions in unsafe mode.

Fri Jul 7 07:20:58 PDT 2023

arsenm added inline comments.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:17037-17040
+    // Avoid folding legal fp_rounds into non-legal ones.
+    if (LegalDAG &&
+        TLI.getOperationAction(ISD::FP_ROUND, VT) != TargetLowering::Legal)
+      return SDValue();
----------------
kosarev wrote:
> This doesn't seem to cause any test failures here upstream and eliminates the need for the True16 f16 = fp_round f64 pattern downstream.
> 
> Not sure if we want that in a separate patch or better keep here to provide some context. It looks problematic to give it a real test without having some True16 support.
> 
I think you're supposed to be checking LegalOperations, not LegalDAG. Use the hasOperation helper?

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:2535-2540
+  if (!TM.Options.UnsafeFPMath)
+    return Helper.lowerFPTRUNC(MI) == LegalizerHelper::Legalized;
+
+  unsigned Flags = MI.getFlags();
+  B.buildFPTrunc(Dst, B.buildFPTrunc(S32, Src, Flags), Flags);
+  MI.eraseFromParent();
----------------
Can move this into the generic code. Also it's unfortunate that we don't have fmf on the cast instructions 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154528