[FPEnv] Document requirement of function attributes with constrained floating point

[kpn]

Summary

We need to document the function attributes that are required for proper functioning of constrained intrinsics.

[uweigand]

I agree with marking function call sites as "strictfp".

But I'm wondering how you came up with this particular list for function definitions? In particular, I'm not sure whether noimplicitfloat is really needed. I understand this flag is supposed to prevent unintended use of floating-point registers (e.g. for Linux kernel code). You don't really need to prevent that for constrained floating point semantics, I think. (You do need to prevent introduction of floating-point instruction that might cause spurious exception, but e.g. introducing moves through FP registers should still be fine.)

As to "strictfp" at the function definition, does this actually do anything currently? I cannot find any LLVM code that checks for this. The documentation also doesn't really say.

[kpn]

In D67839#1681121, @uweigand wrote:

I agree with marking function call sites as "strictfp".

But I'm wondering how you came up with this particular list for function definitions? In particular, I'm not sure whether noimplicitfloat is really needed. I understand this flag is supposed to prevent unintended use of floating-point registers (e.g. for Linux kernel code). You don't really need to prevent that for constrained floating point semantics, I think. (You do need to prevent introduction of floating-point instruction that might cause spurious exception, but e.g. introducing moves through FP registers should still be fine.)

I saw it mentioned in a conversation on the lists at some point. From reading the thread (which I don't have in hand, sorry!) I didn't see anyone disagree. Of course, if it wasn't important enough then maybe there was no good reason to bother at the time. *shrug*

Are we certain that there is no architecture important to LLVM that will ever trap when simply doing a move of arbitrary bytes using FP instructions?

Anybody?

Also, the documentation doesn't say in what circumstances the lack of noimplicitfloat will allow floating point to be introduced. I'm checking on that now. I'm seeing the attribute used in tests with names like "popcnt.ll", so I'm inclined to lean conservative for now at least.

As to "strictfp" at the function definition, does this actually do anything currently? I cannot find any LLVM code that checks for this. The documentation also doesn't really say.

I'm certain that I've seen it mentioned in the context of the inliner, but I don't think anyone has done anything to the inliner yet. I admit I'm unclear on other requirements for it.

[efriedma]

Are we certain that there is no architecture important to LLVM that will ever trap when simply doing a move of arbitrary bytes using FP instructions?

strictfp should be enough to imply that the compiler shouldn't implicitly insert FP operations that can raise an exception, on any target. As it happens, there is one reasonably popular target that can raise an exception on an FP load: x86 without SSE, using x87 floating-point. But noimplicitfloat shouldn't be necessary there... strictfp should be enough to imply that can't insert x87 loads that aren't immediately used by an arithmetic operation. (This is probably broken, currently, and probably nobody will ever fix it, but that isn't really relevant.) No other modern target has similar behavior.

[uweigand]

OK, then I'd suggest we just add "strictfp" to the definition, with the semantics that codegen for this function must not introduce any potentially trapping FP instructions (that were not already in the source IR) -- the latter property should probably also be documented under the "strictfp" entry.

[andrew.w.kaylor]

It was probably me that argued for the strictfp attribute being required on the function definition. My reasoning was that when the inliner wants to inline a function containing strict FP calls into a function that does not, it would use the strictfp attribute as a cue to convert all of the FP operations in the target function to constrained. This can, of course, be deduced from the IR, but if we treat the attribute on the definition as optional then the inliner would always need to check the IR anyway. So if it isn't required it has limited usefulness.

I agree with Eli that strictfp should provide enough information to prohibit introducing FP instructions that might raise exceptions, and that's probably a better argument for requiring the strictfp attribute on function definitions.

[kpn]

Update for review comments. Eliminate verbiage about noimplicitfloat. Add mention of strictfp preventing the addition of potentially trapping instructions.

[andrew.w.kaylor]

This looks good to me.

[kpn]

In D67839#1681121, @uweigand wrote:

I agree with marking function call sites as "strictfp".

Say, would this apply to all function call sites? Or just to ones that aren't intrinsics?

[uweigand]

In D67839#1683038, @kpn wrote:

In D67839#1681121, @uweigand wrote:

I agree with marking function call sites as "strictfp".

Say, would this apply to all function call sites? Or just to ones that aren't intrinsics?

Hmm, I hadn't thought about intrinsics. It probably doesn't matter much -- intrinsics usually already have well-defined semantics, that really cannot be changed by some attribute either. (In particular, the constrained intrinsics already specify their exception semantics; what additional information would adding a strictfp attribute provide?)

[andrew.w.kaylor]

In D67839#1683038, @kpn wrote:

In D67839#1681121, @uweigand wrote:

I agree with marking function call sites as "strictfp".

Say, would this apply to all function call sites? Or just to ones that aren't intrinsics?

I think all.

We've got a problem yet to solve with architecture-specific intrinsics. There are a lot of X86-intrinsics that perform floating point operations. I expect other architecture have the same issue. I doubt that the optimizer attempts to transform many of them, but we'll need to do something when we get to ISel. In theory we could create constrained versions of all of them, but it might be better to just mark them as "strictfp" and have ISel make conservative assumptions. We could attach the local rounding mode and exception behavior as metadata if that would be helpful.

@pengfei , do you think the "strictfp" attribute on an intrinsic would tell you enough for X86 instruction selection? I think for any x86 instructions that take rounding mode operands the intrinsic would already have that explicitly specified, right?

[pengfei]

@pengfei , do you think the "strictfp" attribute on an intrinsic would tell you enough for X86 instruction selection? I think for any x86 instructions that take rounding mode operands the intrinsic would already have that explicitly specified, right?

I think the exception attribute on architecture-specific intrinsics should accept option ignore/maytrap/strict like the constrained intrinsics do. "strictfp" is not enough if we want to attach exception attribute to intrinsic.
For rounding mode, AFAIK, some SSE/AVX intrinsics may implicitly use MXCSR.RC, e.g. __m128i _mm_cvtpd_epi32 (__m128d a), AVX512 intrinsics as well as partial SSE/AVX intrinsics can be specified through parameter, but they still may use MAXCSR.RC if specified with _MM_FROUND_CUR_DIRECTION. Should we attach the rounding attribute to intrinsic, if we had explicitly specified MXCSR.RC?

[gbreynoo]

This is failing on the sphinx build bot:
lab.llvm.org:8011/builders/llvm-sphinx-docs

[kpn]

In D67839#1687976, @gbreynoo wrote:

This is failing on the sphinx build bot:
lab.llvm.org:8011/builders/llvm-sphinx-docs

This should be fixed now. Sorry about the breakage! How do I kick off the llvm-sphinx-docs builder?

llvm/docs/LangRef.rst

 the FP exception status flags, but this mode can also be used with code that
 unmasks FP exceptions.
 
 The number and order of floating-point exceptions is NOT guaranteed.  For
 example, a series of FP operations that each may raise exceptions may be
 vectorized into a single instruction that raises each unique exception a single
 time.
 
+Required Function Attributes:
+"""""""""""""""""""""""""""""
+
+Proper :ref:`function attributes <fnattrs>` usage is required for the
+constrained intrinsics to function correctly.
+
+All function *calls* done in a function that uses constrained floating
+point intrinsics must have the ``strictfp`` attribute.
+
+All function *definitions* that use constrained floating point intrinsics
+must use a set of function attributes. All attributes are required. The
+complete list of required function attributes:
+
+::
+
+      strictfp
+      noimplicitfloat
+
+The constrained floating point intrinsics are still experimental and the above
+list may be subject to change.
+
 
 '``llvm.experimental.constrained.fadd``' Intrinsic
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Syntax:
 """""""
 
 ::