[clang] [llvm] Implement operand bundles for floating-point operations (PR #109798)

Serge Pavlov via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 27 09:25:52 PDT 2024


spavloff wrote:

> I don't think attaching these operand bundles to the existing set of intrinsics is what we want to do. I'd prefer to deprecate the existing intrinsics and define some new scheme that uses these bundles on intrinsics that may be used in other ways.
> 
> One of the original reasons that we talked about the idea of using operand bundles this way is that we wanted a way to indicate strict FP constraints on vector predication intrinsics without requiring an entire second set of intrinsics. I'd like to see a set of generic floating point intrinsics (possibly the existing non-constrained intrinsics) that could be modified with operand bundles in the way you've implemented here, but I guess we'd need some mechanism to ensure that the operand bundles were respected.

Coexistence of old-style intrinsics and operand bundles is a way to make transition to operand bundles gradually. Constrained intrinsics have existed enough long, they have enough extensive support in the compiler and they are already used in production code. Changing the implementation requires vast code changes and making them in several steps  could be less painful. I see the steps:
- operand FP bundles are introduced, metadata arguments still exist,
- intrinsic functions like `experimental_constrained_floor` are replaced by their regular counterparts (like `floor`), their constrained nature is represented by operand bundles only. Intrinsics like `experimental_constrained_fadd` still preserve their original form.
- All remainde constrained functions use operand bundles. Transition is finished.

Eventually operand bundles should remain the only mechanism. It seems it does not make sense to have two mechanisms for the same task.

> I thought bundles were a way to allow us to keep the FP-instruction-constrained intrinsics but dump all the other ones, and the bundles could be attached to any call that needed constrained semantics? I really, really hope we're keeping the safe-by-default semantics of the constrained intrinsics. I can't tell you how important the constrained semantics are to some compiler users like myself, and having to spend the next forever shooting down bugs because the constrained semantics aren't respected in some cases would be seriously damaging.
>
> The IR Verifier changes aren't in place yet in part because there are I don't even remember how many hundreds of places that need to be corrected. A violation of constrained semantics wouldn't be caught by the Verifier (say, if something was hoisted) so we'd never be able to be confident that we got it right and kept it right.

The only mandatory property of any constrained intrinsic is its side effect. It should be enough to maintain constrained semantics. Rounding mode and exception behavior are now only hints. What kind of violation of constrained semantics you observed? Did they relate to wrong ordering? Hundred of violations sound threatening.

> With the constrained intrinsics the default is safe because optimizations don't recognize the constrained intrinsic and thus don't know how to optimize it.

It does not look as a good base for safety. Many users want the code in non-default FP modes to be more efficient, Now any any deviation from the default mode requires use of constrained intrinsics in the entire function, but now such solution exhibits poor performance, unsuitable for practical needs. Eventually optimizations will involve the constrained intrinsics too.

> It seems to me that we also need the ability to represent that the instructions may depend on more than just the dynamic rounding mode for effect, for example denormal flushing bits, exception masking bits, or more exotic target-specific stuff (e.g., x87's precision control bit).

It can be realized using additional bundles type, for example, "fpe.denorm". Or, if we change the bundle representation to metadata, the operand bundles could be like this:
```
    %1 = call double @llvm.func_01(double %0) [ "fpe.modes"(metadata !"rtz", metadata !"daz"), "fpe.except"(metadata !"strict") ]
```


https://github.com/llvm/llvm-project/pull/109798


More information about the cfe-commits mailing list