[FPEnv] Document that constrained FP intrinsics cannot be mixed with non-constrained

[kpn]

Summary

It does not appear that we've documented that constrained FP intrinsics cannot be mixed in a single function with non-constrained instructions. This patch fixes that.

[uweigand]

My understanding was that this is a temporary restriction; in the end, the goal is to allow mixing constrained an non-constrained operations, once we have the necessary barriers. Did I misunderstand this?

Now, I guess it still makes sense to document the restriction even if it is temporary, I'm just wondering whether this should be made explicit in the docs.

[kpn]

In D67360#1667091, @uweigand wrote:

My understanding was that this is a temporary restriction; in the end, the goal is to allow mixing constrained an non-constrained operations, once we have the necessary barriers. Did I misunderstand this?

Now, I guess it still makes sense to document the restriction even if it is temporary, I'm just wondering whether this should be made explicit in the docs.

I can see how knowing that a change to add FP barriers may be coming may affect designs. I believe Serge Pavlov mentioned that he was going to have to "rework" (his word) his patches since he didn't know about the restriction at all. So more information is probably better. I'll update after lunch.

[kpn]

Address review comment. Mention the possibility of the restrictions mentioned here changing in the future, but don't make any promises.

[cameron.mcinally]

In D67360#1667091, @uweigand wrote:

My understanding was that this is a temporary restriction; in the end, the goal is to allow mixing constrained an non-constrained operations, once we have the necessary barriers. Did I misunderstand this?

I think the consensus was that there is no good way to prevent hoisting of non-constrained FP arith ops, since they are side-effect free.

[andrew.w.kaylor]

In D67360#1668007, @kpn wrote:

Address review comment. Mention the possibility of the restrictions mentioned here changing in the future, but don't make any promises.

I don't see any benefit in mentioning that. I agree with Cameron that there is currently no good way to prevent hoisting, and I don't think we want for there to be a way to do that because an explicit goal of the constrained implementation was to ensure that it didn't interfere with optimization of code that didn't require any restrictions, which we believe will always be the majority case by a considerable margin. I expect that mixing of constrained and non-constrained regions will be rather rare. The use of intrinsics is the mechanism we've chosen to restrict code motion.

My understanding of our long term goal is that we will teach optimizations to handle the intrinsics, so that when optimizations are possible (within the constraints described by the intrinsic) they will be performed. Once we reach that state, there will be no serious downside to using an intrinsic with the most permissive settings versus a raw FP operation.

[andrew.w.kaylor]

docs/LangRef.rst

15062

As long as you're cleaning this up, I would suggest ditching the ordinal adjectives (which I never should have used in the original documentation as the problem you're fixing here should have been foreseeable). How about saying "the data operands and the return value" here and referring to "rounding mode argument" and "exception behavior" argument below. You can't really say "next" and "last" there since they aren't always both used.

Some additional rewriting will be necessary to avoid saying "the rounding mode argument is a metadata argument specifying the rounding mode...." Here's a suggestion:

The rounding mode argument is a metadata string specifying what assumptions, if any, the optimizer can make when transforming constant values. Some constrained FP intrinsics omit this argument. If required by the intrinsic, this argument must be one of the following strings:

<...>

The exception behavior argument is a metadata string describing the floating point exception semantics that required for the intrinsic. This argument must be one of the following strings:

[kpn]

Incorporate changes and text requested by Andrew Kaylor. Remove mention of the lack of barriers and include text on how to accomplish mixing without barriers.

[andrew.w.kaylor]

lgtm

Thanks for updating this bit of documentation!

docs/LangRef.rst

 These intrinsics are used to provide special handling of floating-point
 operations when specific rounding mode or floating-point exception behavior is
 required.  By default, LLVM optimization passes assume that the rounding mode is
 round-to-nearest and that floating-point exceptions will not be monitored.
 Constrained FP intrinsics are used to support non-default rounding modes and
 accurately preserve exception behavior without compromising LLVM's ability to
 optimize FP code when the default behavior is used.
 
+If any FP operation in a function is constrained then they all must be
+constrained. This is required for correct LLVM IR. There are currently no
+FP barrier instructions in LLVM IR, and optimizations that move code around
+can create miscompiles if mixing of constrained and normal operations is
+done. This restriction may be relaxed in the future, but for now must be
+followed.
+
 Each of these intrinsics corresponds to a normal floating-point operation.  The
-first two arguments and the return value are the same as the corresponding FP
+first arguments and the return value are the same as the corresponding FP
 operation.
 
-The third argument is a metadata argument specifying the rounding mode to be
+The next argument is a metadata argument specifying the rounding mode to be
 assumed. This argument must be one of the following strings:
 
 ::
 
       "round.dynamic"
       "round.tonearest"
       "round.downward"
       "round.upward"

 transformation is legal for all other rounding modes.
 
 For values other than "round.dynamic" optimization passes may assume that the
 actual runtime rounding mode (as defined in a target-specific manner) matches
 the specified rounding mode, but this is not guaranteed.  Using a specific
 non-dynamic rounding mode which does not match the actual rounding mode at
 runtime results in undefined behavior.
 
-The fourth argument to the constrained floating-point intrinsics specifies the
+The last argument to the constrained floating-point intrinsics specifies the
 required exception behavior.  This argument must be one of the following
 strings:
 
 ::
 
       "fpexcept.ignore"
       "fpexcept.maytrap"
       "fpexcept.strict"