Page MenuHomePhabricator

[strictfp] Add token support for FP constraints
AbandonedPublic

Authored by andrew.w.kaylor on Thu, Nov 14, 11:11 AM.

Details

Summary

This patch adds support for token values to describe floating point rounding modes and exception behaviors. The intent is that we will convert the constrained fp intrinsics to use operand bundles with these tokens instead of string metadata arguments to describe the constraints.

The FPEnv.h header file is borrowed from D69552, which I expect to land before this one. My only changes to the as introduced there are to add "num" entries to the enums.

Diff Detail

Event Timeline

I like the notion of having those values expressed directly as LLVM IR tokens, but I'm not really sure the LLVM "token" type is the correct way to represent those.

The way I understood the token type as described in the IR docs, it seems to be intended to hold opaque numerical values generated at run time by some system library; in the only real current use case, this would be the Windows exception dispatch library.

Using it, like here, to encode LLVM internal compile-time enumeration values seems to be a bit of an overloading of that concept. I think this may probably need to be discussed more with the wider community, maybe on llvm-dev?

I like the notion of having those values expressed directly as LLVM IR tokens

+1
I also like the (optional) operand bundles (see llvm-dev reply).

Quoting: https://llvm.org/docs/LangRef.html#token-type

"The token type is used when a value is associated with an instruction but all uses of the value must not attempt to introspect or obscure it. As such, it is not appropriate to have a phi or select of type token."

That seems to be a pretty accurate description of how the fpround/fpexcept params are supposed to be handled (they must not change the operation/implementation of the constrained fp op).

Quoting: https://llvm.org/docs/LangRef.html#token-type

"The token type is used when a value is associated with an instruction but all uses of the value must not attempt to introspect or obscure it. As such, it is not appropriate to have a phi or select of type token."

That seems to be a pretty accurate description of how the fpround/fpexcept params are supposed to be handled (they must not change the operation/implementation of the constrained fp op).

Maybe I'm just interpreting this differently, but for me this reads as a description of the token value itself, which is something opaque that the compiler must not attempt to modify (as in the current use of tokens with Windows exception handling, where the token values generated by the OS apparently interact with exception control flow in a non-obvious way).

This is really a different scenario from constrained fp ops, where the fpround/fpexcept params are not so much operands, but just compile-time constants associated with an instruction -- in a sense, more like "extended opcodes" (similar to fast-math flags or the like).

As referenced in Simon's reply, I have started a mailing list discussion about this.

I see your point, Ulrich, about this overloading the token concept a bit. The token type is also used by coroutine handling, but it uses tokens in the same way as exception handling -- it's an opaque value returned by an intrinsic call and passed to other intrinsics. In what I've proposed here, the optimizer is intended to inspect and use the value of the token. However, it's still opaque to the program. For instance, you can't assign the token to another token or add two tokens together or print the token to stdout.

I feel pretty good about moving the constraints to an operand bundle and making it optional. I'm on the fence about tokens as the way to do it.

I don't think any current concept fits better (maybe attributes, but call site specific attributes are uncommon and confusing). A new IR concept to handle this feels like a worse idea than extending the concept of tokens.

andrew.w.kaylor abandoned this revision.Wed, Dec 4, 11:59 AM

I think we've agreed that this isn't a good use of tokens. The operand bundle approach may still be useful, but that can be attempted in a new revision.