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

[Xiang Zhang via Phabricator via llvm-commits]

xiangzhangllvm added a comment.

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

> Why does this patch not touch the code in DAGTypeLegalizer::PromoteIntRes_FP_TO_XINT? If the code in LegalizeVectorOps is a problem I think that code is too.

Let me fix here too, thanks very much for your remind!



================
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
----------------
efriedma wrote:
> 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.)
Sorry, not much understand, Here the test didn't use poison elements, and we don't know the fptoui will be overflow or not, it is runtime.




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

https://reviews.llvm.org/D106053