[PATCH] D84238: [PowerPC] Set v1i128 to expand for SETCC to avoid crash

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang added a comment.

In D84238#2166241 <https://reviews.llvm.org/D84238#2166241>, @ZhangKang wrote:

> In D84238#2165944 <https://reviews.llvm.org/D84238#2165944>, @steven.zhang wrote:
>
> > I still think there is something missing here. What you set the legalize action for SETCC is under P8 <https://reviews.llvm.org/P8> altivec, but why the case passed for P7 <https://reviews.llvm.org/P7> ? And this case passed without your patch :
>
>
> P7 <https://reviews.llvm.org/P7> will convert result from vector `v1i128 setcc` to  two scalar `setcc i64`.
>
> >   define <1 x i1> @setcc_v1i128(<1 x i128> %a) {
> >   entry:
> >     %0 = icmp ult <1 x i128> %a, <i128 35708>
> >     ret <1 x i1> %0
> >   }
> > 
> > 
> > The only difference I see is that, it is `v1i1 = setcc t47, t74, setult:ch` instead of `v1i128 = setcc t47, t74, setult:ch`. So, what happens here ?
>
> If the return value is <1 x i1>, the result will be converted to scalar `i1 = setcc i128`.
>  Using below IR, you will get the same assertion error this patch mentioned. Because the return value is <1 x i64>, 
>  the result will not be converted to scalar.
>
>   define <1 x i64> @setcc_v1i128(<1 x i128> %a) {
>   entry:
>     %0 = icmp ult <1 x i128> %a, <i128 35708>
>     %1 = zext <1 x i1> %0 to <1 x i64>
>     ret <1 x i64> %1
>   }
>


The new test makes more sense to me. Can you please use this one ? So,  yes, mark the v1i128 as expand so that, we won't have any setcc with v1i128 after type legalization, which fix the crashing you see when we are trying to select an opcode for v1i128.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84238