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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

In D106053#2885066 <https://reviews.llvm.org/D106053#2885066>, @xiangzhangllvm wrote:

> In D106053#2885026 <https://reviews.llvm.org/D106053#2885026>, @efriedma wrote:
>
>>> +  After appending AssertZext, the shuffle optimization will stop zero these elements, and directly use them.   That is wrong.
>>
>> I agree up to this point.
>>
>> The question is, is the AssertZext node wrong, or are the shuffle optimizations wrong?  That dictates whether we go with this patch, or instead revert 2a419a0b9957 <https://reviews.llvm.org/rG2a419a0b9957ebac9e11e4b43bc9fbe42a9207df> (and any similar optimizations, if they exist).
>
> It is the "adding AssertZext node" wrong. AssertZext node should correctly marked node which is really zero in N bits.

A lane of a vector is either poisoned, or not poisoned.  It doesn't apply to individual bits of a vector.

Extending this to how computeKnownBits works, if computeKnownBits says a bit is "known", it doesn't really mean the bit has to have that value.  It means either the bit has that value, or that lane of the vector is poisoned.  As far as I know, this applies to both the SelectionDAG and the ValueTracking versions of computeKnownBits.



================
Comment at: llvm/test/CodeGen/X86/fptoui-may-overflow.ll:5
+; We can't only generate "cvttps2dq %xmm0, %xmm0" for this function, because
+; there may be overflow case in fptoui. The old DAG optimization will optimize
+; "%f = fptoui <4 x float> %arg to <4 x i8>" to
----------------
aqjune wrote:
> According to LangRef (https://llvm.org/docs/LangRef.html#fptoui-to-instruction), the overflow returns `poison` which means that using the value is invalid.
> 
> This is analogous to using an uninitialized variable in C/C++; there is no guarantee that the compiled program will have a reasonable behavior.
> ```
> int x; // not initialized
> printf("%d", x); // Assume that this printed 0xDEADBEEF
> ... (don't update x)
> printf("%d", x); // There is no guarantee that this will also print 0xDEADBEEF
> ```
The comment is wrong.  But the CHECK lines are correct.  LangRef and alive2 say the following transform is invalid:

```
define <16 x i8> @src(<4 x float> %arg1) {
  %ss = shufflevector <16 x i8> poison, <16 x i8> zeroinitializer, <16 x i32> <i32 0, i32 17, i32 18, i32 19, i32 1, i32 21, i32 22, i32 23, i32 2, i32 25, i32 26, i32 27, i32 3, i32 29, i32 30, i32 31>
  ret <16 x i8> %ss
}
=>
define <16 x i8> @tgt(<4 x float> %arg1) {
  ret <16 x i8> poison
}
```

(On a side-note, alive2 gives some confusing results for fptoui; apparently it thinks `fptoui float 31.5 to i32` is poison.)


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

https://reviews.llvm.org/D106053