[PATCH] D70226: Add an option to disable strict float node mutating to an normal float node

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

> In my opinion, the behavior currently is reasonable. I don't think there's a way in common code can handle an expand strict node if its non-strict node is custom. Otherwise, its non-strict node isn't necessarily to be custom.

What I was thinking about is: there are some operations where we can have an Expand implementation that correctly respects strict semantics, typically by mapping on top of (other) strict operations.  E.g. an implementation of STRICT_UINT_TO_FP in terms of STRICT_SINT_TO_FP (see discussion in D69275 <https://reviews.llvm.org/D69275>).   In those cases, the target should be able to select that expansion, even if it has a UINT_TO_FP custom handler (for whatever reason, maybe for some optimizations that aren't possible in the strict case).  There's no particular reason to disallow this.

> And for target that isStrictFPEnabled, the action of the strict nodes can not be set to expand if the expansion does not respect constrained FP semantics.

But it may still be possible to respect strict semantics by expanding to a libcall -- and that is what should happen in those cases, I think.

In summary, there are four potential cases how Expand of a STRICT node could be implemented:

1. A custom expansion sequence that respects constrained semantics
2. Expansion to libcall (where the library is assumed to respect constrained semantics)
3. A custom expansion sequence that **does not** respect constrained semantics
4. "Fake" expansion to the non-strict node

If isStrictFPEnabled is true, then cases 3) and 4) above are forbidden, but cases 1) and 2) are still OK.

My main point is that the difference between 1) and 3) has to be determined on a case-by-case basis by inspecting the particular expansion sequence, and therefore this should be checked in-line in each expansion sequence.  This is why I'd prefer to have each affected custom expansion sequence implementation directly the flag check whether or not strict semantics must be enforced or not; I don't believe this can be a single check just at the top of ExpandNode.


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

https://reviews.llvm.org/D70226