[PATCH] D101481: [AMDGPU] Select V_CVT_*16_F16 more often

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

All of the patterns you added look to me like workarounds for the DAGCombiner or legalizer not doing its job correctly



================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:918
+def : GCNPat <
+  (i16 (fp_to_sint f16:$src)),
+  (V_CVT_I16_F16_e32 VSrc_b32:$src)
----------------
This is identical to the pattern attached to the instruction definition, so this shouldn't be doing anything


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:923
+def : GCNPat <
+  (i16 (fp_to_uint f16:$src)),
+  (V_CVT_U16_F16_e32 VSrc_b32:$src)
----------------
Ditto


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:928-929
+def : GCNPat <
+  (i16 (fp_to_sint f32:$src)),
+  (V_CVT_I32_F32_e32 VSrc_b32:$src)
+>;
----------------
I don't see the advantage of just selecting the 16-bit result op to the 32-bit result. The legalizer should just take care of this


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:933
+def : GCNPat <
+  (i16 (fp_to_uint f32:$src)),
+  (V_CVT_U32_F32_e32 VSrc_b32:$src)
----------------
Ditto


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:938
+def : GCNPat <
+  (i16 (fp_to_sint f64:$src)),
+  (V_CVT_I32_F64_e32 VReg_64:$src)
----------------
Ditto


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:943
+def : GCNPat <
+  (i16 (fp_to_uint f64:$src)),
+  (V_CVT_U32_F64_e32 VReg_64:$src)
----------------
Ditto


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:949-955
+  (i32 (sext (i16 (fp_to_sint f16:$src)))),
+  (V_LSHLREV_B32_e32 (i32 16), (V_CVT_I16_F16_e32 VSrc_b32:$src))
+>;
+
+def : GCNPat <
+  (i32 (sext (i16 (fp_to_uint f16:$src)))),
+  (V_LSHLREV_B32_e32 (i32 16), (V_CVT_U16_F16_e32 VSrc_b32:$src))
----------------
Would these sexts ever reach the selector? I would expect the combiner to take care of these. I don't see tests for this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101481