[PATCH] D93455: Constrained fp OpBundles

[Simon Moll via Phabricator via llvm-commits]

simoll added a comment.

In D93455#2460765 <https://reviews.llvm.org/D93455#2460765>, @kpn wrote:

> Do you need tests for the verifier part?

Probably. With this patch standalone, we can only do "catch-22 testing": all uses of the bundle are illegal. We can extend that once the VP floating-point intrinsics are upstream that actually support these bundles.

> Would replacing some of the constrained intrinsics with operand bundles on other intrinsics have the same safety guarantee in the middle end? Currently the middle end doesn't know about the constrained intrinsics so it handles code conservatively.

Not easily. This has the same re-occurring issue as enabling predication on all instructions: if we allow the constrained bundle on regular fp intrinsics overnight, existing optimization code will ignore the bundles and potentially break code.
An elegant way out for this would be to introduce an `FPMathIntrinsic` helper class that only matches intrinsics in the default fp environment (that is without the bundle). All code that looks for a specific fp math function can then no longer just take the intrinsic id but needs to cast the intrinsic to that helper class first.
Btw, the `FPMathIntrinsic` helper also fits well into the generalized pattern matching framework (D92086 <https://reviews.llvm.org/D92086>).

> What about intrinsics the middle end already knows about?

Constrained bundles and the existing constrained intrinsics can exist side by side if all places that reason about constrained fp do so through the `ConstrainedFPIntrinsic` class. Once that is the case and we want to switch on bundles on some intrinsics, we only need to wire that up in `ConstrainedFPIntrinsic` .

> Would the new operand bundle be as safe as the constrained intrinsics are now?

Yes, it is illegal to drop operand bundles (see the LangRef <https://llvm.org/docs/LangRef.html#operand-bundles>) . The bundle is just as stable as the regular operand list of the intrinsics. One thing to take care of is proper use of callsite attributes: when the bundle is used with non-default settings, the call site must have the `strictfp` attribute and potentially others depending on the fp configuration (eg it may trap, access memory (fp config registers), ..).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93455/new/

https://reviews.llvm.org/D93455