[PATCH] D110579: [AMDGPU] Add two new intrinsics to control fp_trunc rounding mode

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

I'm not sure what level of approval is needed for new experimental intrinsics. It might be worth emailing llvmdev to explain the requirement. In any case they need to be documented in `docs/LangRef.rst`.

You're using EmitInstrWithCustomInserter to emit s_round_mode instructions, and then adding a new phase to SIModeRegister to remove them. This seems wrong. You should be able to teach SIModeRegister to insert the required s_round_mode instructions.



================
Comment at: llvm/include/llvm/IR/Intrinsics.td:905
+
+def int_experimental_fptrunc_round_upward : DefaultAttrsIntrinsic<[ llvm_anyfloat_ty ],
+                                                    [ llvm_anyfloat_ty]>;
----------------
Needs a comment.


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:908
+
+def int_experimental_fptrunc_round_downward : DefaultAttrsIntrinsic<[ llvm_anyfloat_ty ],
+                                                    [ llvm_anyfloat_ty ]>;
----------------
Needs a comment.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:182-183
+let usesCustomInserter = 1, hasSideEffects = 0, mayLoad = 0, mayStore = 0, Uses = [MODE, EXEC] in {
+def FPTRUNC_UPWARD_PSEUDO : SPseudoInstSI <(outs unknown:$sdst), (ins unknown:$src0)>;
+def FPTRUNC_DOWNWARD_PSEUDO : SPseudoInstSI <(outs unknown:$sdst), (ins unknown:$src0)>;
+} // End let usesCustomInserter = 1, hasSideEffects = 0, mayLoad = 0, mayStore = 0, Uses = [MODE, EXEC]
----------------
Can you implement instruction selection for the intrinsics by adding patterns here, instead of writing C++ code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110579