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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:907
+def int_fptrunc_round : DefaultAttrsIntrinsic<[ llvm_anyfloat_ty ],
+                                                    [ llvm_anyfloat_ty, llvm_metadata_ty ]>;
+
----------------
Align the brackets with the previous line.


================
Comment at: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:2261
+    // Convert the metadata argument to a constant integer
+    Metadata *MD = dyn_cast<MetadataAsValue>(CI.getOperand(1))->getMetadata();
+    Optional<RoundingMode> RoundMode =
----------------
Use cast instead of dyn_cast if it can't fail. Or check that the dyn_cast didn't return null if it can fail.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6337
+    const Function *F = I.getCalledFunction();
+    bool HasChain = !F->doesNotAccessMemory();
+    bool OnlyLoad = HasChain && F->onlyReadsMemory();
----------------
Why would this intrinsic ever have a chain?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6349
+    // call
+    Metadata *MD = dyn_cast<MetadataAsValue>(I.getOperand(1))->getMetadata();
+    Optional<RoundingMode> RoundMode =
----------------
Use cast instead of dyn_cast if it can't fail. Or check that the dyn_cast didn't return null if it can fail.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6384
+    SDValue Result;
+    Result = DAG.getNode(ISD::INTRINSIC_W_CHAIN, getCurSDLoc(), VTs, Ops);
+
----------------
The code has optionally added a chain, but INTRINSIC_W_CHAIN must always have a chain. Whether there is a chain or not is a property of the ISD opcode.


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

https://reviews.llvm.org/D110579