[PATCH] D43142: Experimental pass to convert all floating point operations to the equivalent constrained intrinsics

Andy Kaylor via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 2 11:07:35 PDT 2018


andrew.w.kaylor added a comment.

In https://reviews.llvm.org/D43142#1285069, @cameron.mcinally wrote:

> I'm realizing that FENV_ACCESS is poorly designed.


Yes, this is true. It's difficult to understand and difficult to implement. Personally, I'm not as worried about precisely supporting FENV_ACCESS as I am supporting the code behavior that it is intended to enable. The FENV_ACCESS pragma is useful as a reference, but for me this is really about being able to control the optimizer and allow a way to restrict what it does with FP operations. The optimizer has no business knowing anything about pragmas in the source code and what any given language standard says about them. It just needs to be internally consistent and follow the rules we set for the IR language.

But getting back to FENV_ACCESS (because I would like to see us provide a correct implementation of the standard) as it applies to your example, I believe the standard says that the compiler should assume the default environment for any region marked as FENV_ACCESS OFF. The behavior is only undefined if the FP environment doesn't match the defaults when that code is executed. This puts the burden of making sure the environment is correct for transitions between regions with different access settings. That's a bit of a nightmare for the programmer if he decides to mix FENV_ACCESS modes across a program, but it's fairly simple for the compiler. In your original example, we are free to constant fold the division in bar and not worry about the fact that the FP exception won't occur.

Furthermore, if we inline bar into main, we can use "round.tonearest" as the rounding mode argument and "fpexcept.ignore" as the exception behavior argument, so we could still constant fold the division if we hadn't done so previously for whatever reason. This is because the contents of bar are still conceptually in the FENV_ACCESS OFF scope as the user originally specified. The fact that we have chosen to inline the function doesn't change the semantics. The only thing we need to worry about is making sure that FP operations don't migrate across calls or other FP operations that would break these semantics. Using the intrinsics after inlining takes care of that.

In https://reviews.llvm.org/D43142#1285069, @cameron.mcinally wrote:

> Let's take this small example:
>
>   #include <fenv.h>
>   #include <stdio.h>
>  
>   void bar();
>  
>   #pragma STDC FENV_ACCESS ON
>   int main() {
>     feenableexcept(FE_DIVBYZERO);
>     bar();
>     float x = 1.0/0.0;
>     printf("main x=%lf\n", x);
>   }
>  
>   #pragma STDC FENV_ACCESS OFF
>   void bar() {
>     float x = 1.0/0.0;
>     printf("bar x=%lf\n", x);
>   }
>
>
> The FDiv in bar() would access the FP environment since main() enabled exceptions. But, since bar() is preceded with FENV_ACCESS=OFF, the Standard says that accessing the FP environment in bar() is undefined behavior. Since it's undefined behavior, the compiler can optimize as it wishes, including inlining bar() without modifying the FDiv.
>
> It would really be the user's responsibility to make bar() safe. E.g.:
>
>   #pragma STDC FENV_ACCESS OFF
>   void bar() {
>     fedisableexcept(FE_DIVBYZERO);
>     float x = 1.0/0.0;
>     printf("bar x=%lf\n", x);
>     feenableexcept(FE_DIVBYZERO);
>   }
>
>
> In that case, we would be free to inline bar() assuming that the feenableexcept/fedisableexcept functions act as barriers. However, explicitly making each call safe seems like an unreasonable burden on the user. Imagine the mental gymnastics necessary for a large call tree.
>
> I suppose the compiler could automatically manage this for the user, by saving/restoring the FP environment in the prolog/epilog of FENV_ACCESS=OFF functions. But, there would be a non-trivial cost for that. A priori reasoning tells me that it's not worth the trouble.
>
> Can anyone poke holes in this?





https://reviews.llvm.org/D43142





More information about the llvm-commits mailing list