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

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 3 08:02:37 PST 2021


spatel added subscribers: nikic, bjope.
spatel added a comment.

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?


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

https://reviews.llvm.org/D114964



More information about the llvm-commits mailing list