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

[Julien Pagès via Phabricator via llvm-commits]

jpages added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6337
+    const Function *F = I.getCalledFunction();
+    bool HasChain = !F->doesNotAccessMemory();
+    bool OnlyLoad = HasChain && F->onlyReadsMemory();
----------------
foad wrote:
> craig.topper wrote:
> > craig.topper wrote:
> > > Why would this intrinsic ever have a chain?
> > Err. I guess the intrinsic attributes are defined to always have a Chain. Should it be IntrNoMem instead?
> IIUC, if the rounding mode is allowed to be "dynamic" then the intrinsic needs to be IntrInaccessibleMemOnly (just like the constrained fp intrinsics). If we don't allow "dynamic" then it could be IntrNoMem.
> 
> Personally I have no interest in supporting "dynamic" so I would vote for disallowing it and making the intrinsic IntrNoMem.
Yes the original version was defined to have a chain, I changed it with IntrNoMem/INTRINSIC_WO_CHAIN as suggested. Thank you for the suggestions.

As a consequence this code is a bit simpler, and the lowering in the backend was modified to reflect that .


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

https://reviews.llvm.org/D110579