[PATCH] D106053: [CodeGen] Remove pending AssertZext AssertSext in promoting FP_TO_INT

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D106053#2881905 <https://reviews.llvm.org/D106053#2881905>, @craig.topper wrote:

> In D106053#2881878 <https://reviews.llvm.org/D106053#2881878>, @LuoYuanke wrote:
>
>> In D106053#2881182 <https://reviews.llvm.org/D106053#2881182>, @efriedma wrote:
>>
>>> To be clear, I'm working on the assumption that AssertZext means "if the operand is not poison, the high N bits are not set".  I'm not sure we've formally stated that anywhere, but I think it's consistent with how SelectionDAG optimizations use AssertZext in practice.
>>
>> Is it possible that user expect some saturate behaviour (get the max value of the int) when convert from FP to INT overflow? AssertZext cause the undefined behaviour be propagated to each optimization that assume the upper bits being zero.
>
> It's undefined behavior according to C. The code would get different behavior on different platforms and shouldn't be relied on. We have fptosi_sat and fptoui_sat intrinsics that are used by rust that produce a well defined behavior on overflow. It doesn't look like we have a target independent builtin for them.

I'm not sure you get saturation with this assert removed. The result will be truncated to the original integer type. If the conversion overflowed the original type but not int32, you would just get the lower 8 or 16 bits of the conversion result. If it overflowed the conversion to int32, the cvt instruction produces INT_MIN and the truncate will turn it into 0.


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

https://reviews.llvm.org/D106053