[PATCH] D43515: More math intrinsics for conservative math handling

Tue Mar 6 09:14:36 PST 2018

andrew.w.kaylor added inline comments.

================
Comment at: docs/LangRef.rst:13337
+      declare <type>
+      @llvm.experimental.constrained.extend(<type> <op>,
+                                          metadata <rounding mode>,
----------------
kpn wrote:
> kpn wrote:
> > andrew.w.kaylor wrote:
> > > kpn wrote:
> > > > andrew.w.kaylor wrote:
> > > > > This is a replacement for fpext, right? I think you should say that somewhere.
> > > > I think I picked names for the intrinsics that matched the SelectionDAG node enum. Perhaps it would be better to match the bitcode language names? In which case this intrinsic would be "fpext" instead of "extend".
> > > > 
> > > > Either way that's a good idea for the documentation to at least mention fpext.
> > > These intrinsics should be driven by what the front end needs. If no front end is generating an equivalent now then we don't want an intrinsic. So, yes, please match the bitcode language names.
> > Will do.
> Say, now that I think about this, what does rounding have to do with extending a FP value? Shouldn't this intrinsic just plain not have rounding metadata?
That's a reasonable point. The same issue came up with frem. The rounding mode doesn't apply to frem, but I kept the argument just so that it could be handled internally the same way as the other constrained intrinsics and then documented in the language reference that the rounding mode has no effect. I'm not completely convinced that was a good decision on my part.

If you want to look at what would have to change to support constrained intrinsics with no rounding mode argument, I would likely support that. I don't think it would be much extra handling. There are just some things where the class that is used to represent the intrinsics (ConstrainedFPIntrinsic) would need to be aware of the possibility that this argument is omitted.

Repository:
  rL LLVM

https://reviews.llvm.org/D43515