[PATCH] D114964: [DAG] Create fptoui.sat from clamped fptoui

Bjorn Pettersson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 3 08:35:02 PST 2021


bjope added a comment.

In D114964#3169854 <https://reviews.llvm.org/D114964#3169854>, @spatel wrote:

> In D114964#3168031 <https://reviews.llvm.org/D114964#3168031>, @dmgreen wrote:
>
>> An fptoui.sat is more defined than a fptoui + umin. For the fptoui any out-of-range value produces poison - for the fptoui.sat the out of range values are defined to saturate. So the transform isn't reversible and on some architectures produces worse code.
>
> Ah, right. This is similar to a problem we have with funnel-shift intrinsics. After several improvements in the default expansion, we are able to canonicalize most patterns, but there are still a few that could escape because the safe expansion has more instructions than the unsafe pattern on targets that don't have good shift/rotates.
>
>> I would actually really like it to be done in IR. We would need to vectorize these if we can, and they are much simpler to vectorize if we already have the scalar instructions. But it needs to be a costed decision, not an inst-combine canonicalization decision. Any ideas of a good place to make that happen?
>
> Yes, trying to bend the cost models to recognize larger patterns like this is tough. VectorCombine does generic transforms using costs, but this doesn't quite fall into that category...
>
> cc'ing @nikic @bjope based on the history:
> https://groups.google.com/g/llvm-dev/c/cgDFaBmCnDQ/m/CZAIMj4IBAAJ
> D54749 <https://reviews.llvm.org/D54749>
>
> I'm not sure if there was a plan for these intrinsics to be used from C-like source. Maybe we need an fp-combine pass?

My use case for these were to related to the Embedded-C support, to implement conversion between fixed point types and floating point types. This was added in https://reviews.llvm.org/D86632. In that patch the intrinsics are used directly when producing IR in the frontend.

Downstream we set FP_TO_SINT_SAT as "custom" as we can do optimized lowering in some situation (depending on involved types and saturation width). I realize that we probably want to override shouldConvertFpToSat to avoid any conversion to these saturated intrinsics when it isn't beneficial for our target.


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

https://reviews.llvm.org/D114964



More information about the llvm-commits mailing list