[PATCH] D55897: Add constrained fptrunc and fpext intrinsics

Kevin P. Neal via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 26 07:58:39 PDT 2019


kpn added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2787
+  case ISD::STRICT_FP_ROUND:
+    Tmp1 = EmitStackConvert(Node->getOperand(1), 
+                            Node->getValueType(0),
----------------
andrew.w.kaylor wrote:
> Can this cause the wrong rounding mode to be used? For X86 targets I would guess it will result in an instruction that uses the runtime rounding mode, but I'm not sure we can count on that for all targets. Also, if the value being converted is a constant, might this get folded using the default rounding mode? And if it is a constant and we knew the rounding mode based on an argument to the intrinsic, we might want to fold it (though ideally that would have happened before this).
Isn't this an argument for a strict load node types and maybe a strict store type as well? 

I'd be surprised if we were doing constant folding of a store+load combination, but if we are then either a strict load or store node would disable that folding.

Strictly speaking, using the runtime rounding mode would be incorrect if that wasn't what was specified in the rounding mode field of the intrinsic. So even X86 could be wrong.

We aren't doing anything with the rounding and exception arguments yet for any of the new intrinsics. So how about I file a bug noting that fact and noting that we need to not forget about this case? Then we can move on and come back later.


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

https://reviews.llvm.org/D55897





More information about the llvm-commits mailing list