[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Cameron McInally via Phabricator via cfe-commits]

cameron.mcinally added a comment.

In https://reviews.llvm.org/D53157#1301992, @hfinkel wrote:

> > Just because FENV_ACCESS can be toggled on that granularity doesn't mean we have to represent it that way. We've previously agreed (I think) that if FENV_ACCESS is enabled anywhere in a function we will want to use the constrained intrinsics for all FP operations in the function, not just the ones in the scope where it was specified.
>
> Yes, this is also my understanding. We can't soundly mix the two in the same function because we can't prevent the code motion within the function.


Ugh, I don't know. The C Standard's language is so vague.

In https://reviews.llvm.org/D53157#1301994, @hfinkel wrote:

> The rounding mode does need to be reset to its default setting when passing from FENV_ACCESS "on" to FENV_ACCESS "off", but that seems to be the user's responsibility. Are you saying that the implementation should reset it on that transition?


Yes, that's how I was interpreting the Standard (today). The implementation should reset the control modes. The verbiage is murky at best though.

We touched on this in https://reviews.llvm.org/D43142 and I do realize that my opinion has flip-flopped since then. I previously believed that reseting the control modes was up to the user, but now I'm not so sure. I suppose that either way, as long as a fesetround(default_mode_constant) is seen with a FENV_ACCESS=OFF, we could use that as a barrier to prevent the problematic code motion.

Thinking aloud, maybe we should be working on redefining FENV_ACCESS in the C Standard? It's pretty clear that this section could use some work.

All that said, my understanding of $7.6.1 in the Standard is cloudy at best. If I'm the only one that feels this way, I'll drop my objections...


https://reviews.llvm.org/D53157