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

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added a comment.

In D106053#2882183 <https://reviews.llvm.org/D106053#2882183>, @efriedma wrote:

> If I'm understanding correctly, the testcase reduces to something like this:
>
>   define <16 x i8> @src(<4 x float> %arg1) {
>     %f = fptoui <4 x float> %arg1 to <4 x i8>
>     %s = shufflevector <4 x i8> %f, <4 x i8> undef, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef>
>     %ss = shufflevector <16 x i8> %s, <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) {
>     %f = fptosi <4 x float> %arg1 to <4 x i32>
>     %ss = bitcast <4 x i32> %f to <16 x i8>
>     ret <16 x i8> %ss
>   }

Other than reverting the patch, can `%f` in tgt be frozen? (Unless it is likely to cause performance regression; I found that updating vector ops to deal with freeze is hard work)?



================
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
----------------
xiangzhangllvm wrote:
> 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.
> 
> 
@efriedma Thank you for the report, I made a pull request at Alive2 that fixes the bugs. @nlopes will be back in a few weeks and have a look at it.

@xiangzhangllvm poison is kind of a conceptual value that appears in LLVM IR's abstract machine. 
It is used to carry guarantees from C/C++ that e.g., casting big floats to signed integer is not legal.
Of course, one cannot statically determine if a C program will do such cast or not.
As double free raises segmentation fault, the execution can fail or print a bogus value; integer overflow is just less visible to users.


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

https://reviews.llvm.org/D106053