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

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D53157#1302159, @cameron.mcinally wrote:

> 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.


To be clear, I mean here that *we* can't mix the two soundly in the same function (where we generate some operations using the constrained instrinsics and some not) because of constraints imposed by our design. The standard may indeed allow for more precision. We'll need to be conservatively correct.

> 
> 
> 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.

I'll also point out that whether or not we insert a rounding-mode reset when the pragma changes state is orthogonal to whether we stop emitting constrained intrinsics at that point.

> 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...

I don't read the standard that way, but the standard could certainly be more clear.


https://reviews.llvm.org/D53157