[PATCH] D98410: [ConstantFold] Handle undef/poison when constant folding smul_fix/smul_fix_sat

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

In D98410#2619538 <https://reviews.llvm.org/D98410#2619538>, @nagisa wrote:

> In D98410#2619315 <https://reviews.llvm.org/D98410#2619315>, @bjope wrote:
>
>> So undef include values that isn't possible for every combination of the other operands.
>
> Aha, I see! This is an interesting reasoning. I think it is fine to do what you're doing for the sake of simplicity, what follows under the horizontal line below is a bunch of my rambling (feel free to ignore all of it)
>
> ---
>
> Taking a regular `mul` instruction as an example
>
>   mul i32 1, undef  # => undef
>   mul i32 2, undef  # => 0
>   mul i32 3, undef  # => undef
>
> These make sense to me, though require some roundabout reasoning. `1 * undef = undef` is trivial. `2 * undef` follows the same reasoning as yours in that there's bit-pattern `undef` can take that would result in lower bit set here. That same reasoning could apply for `3 * undef` too – but it does not because of wrap-around (I think).
>
> And this is where the following snippet from the smul.fix <https://llvm.org/docs/LangRef.html#llvm-smul-fix-intrinsics> definition comes in:
>
>> It is undefined behavior if the result value does not fit within the range of the fixed point type.
>
> Basically, unless the intrinsic is multiplying `undef` by `1` or by `0`, I believe there's an implicit invocation of UB. That's because there exist bit patterns of an `undef` operand that multiplied by other values would cause the resulting value to not fit within the fixed point type's range (right?). Whether we manifest this UB as `undef` or `0` probably doesn't matter too much, but I //think// either option would be valid.

It depends a bit on the scale. If you for example multiply by 0.5 then you won't get any overflow. On the other hand, smul.fix with scale=0 is pretty much like a regular mul but not sure we need to make a special case for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98410