[PowerPC] Set v1i128 to expand for SETCC to avoid crash

[ZhangKang]

Summary

PPC only supports the instruction selection for v16i8, v8i16, v4i32, v2i64, v4f32 and v2f64 for ISD::SETCC,
don't support the v1i128, so below IR will crash:

t35: v1i128 = setcc t47, t74, setult:ch

This patch is to set v1i128 to expand for SETCC to avoid crash. And the expanding can get the expected result.

[lkail]

llvm/test/CodeGen/PowerPC/setcc-vector.ll

5

Can you add RUN lines for pwr8 since it's guarded by hasP8Altivec?

[ZhangKang]

Add more test cases.

[steven.zhang]

Your test didn't crash even without your fix.

[ZhangKang]

In D84238#2165867, @steven.zhang wrote:

Your test didn't crash even without your fix.

It needs ASSERT on. I have tried it again, it will hit below assertion error:

Invalid integer vector compare condition
UNREACHABLE executed at /home/ken/llvm/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:4044!

[lkail]

llvm/test/CodeGen/PowerPC/setcc-vector.ll

7

Should remove le or it will confuse developers since pwr7 doesn't feature little-endian.

[ZhangKang]

Remove le for PWR7.

[steven.zhang]

I still think there is something missing here. What you set the legalize action for SETCC is under P8 altivec, but why the case passed for P7 ? And this case passed without your patch :

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 ?

[ZhangKang]

In D84238#2165944, @steven.zhang wrote:

I still think there is something missing here. What you set the legalize action for SETCC is under P8 altivec, but why the case passed for P7 ? And this case passed without your patch :

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
}

[steven.zhang]

In D84238#2166241, @ZhangKang wrote:

In D84238#2165944, @steven.zhang wrote:

I still think there is something missing here. What you set the legalize action for SETCC is under P8 altivec, but why the case passed for P7 ? And this case passed without your patch :

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.

[ZhangKang]

Update the test case.

[ZhangKang]

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.

Have updated the test case.

[ZhangKang]

Remove the hasP8Vector condition.

[steven.zhang]

LGTM now. Thank you for fixing this.

[hans]

Pushed to 11.x as 7569e8c696288cd9c9409936b4fe9b846d0bd0b7 to fix https://bugs.llvm.org/show_bug.cgi?id=47374

Please let me know if there are any follow-ups.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

if (Subtarget.hasVSX()) {
 
         setOperationAction(ISD::SETCC, MVT::v2i64, Custom);
 
         // VSX v2i64 only supports non-arithmetic operations.
         setOperationAction(ISD::ADD, MVT::v2i64, Expand);
         setOperationAction(ISD::SUB, MVT::v2i64, Expand);
       }
 
+      setOperationAction(ISD::SETCC, MVT::v1i128, Expand);
+
       setOperationAction(ISD::LOAD, MVT::v2i64, Promote);
       AddPromotedToType (ISD::LOAD, MVT::v2i64, MVT::v2f64);
       setOperationAction(ISD::STORE, MVT::v2i64, Promote);
       AddPromotedToType (ISD::STORE, MVT::v2i64, MVT::v2f64);
 
       setOperationAction(ISD::VECTOR_SHUFFLE, MVT::v2i64, Legal);
 
       setOperationAction(ISD::SINT_TO_FP, MVT::v2i64, Legal);

llvm/test/CodeGen/PowerPC/setcc-vector.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \
+; RUN:   -ppc-asm-full-reg-names < %s | FileCheck -check-prefixes=CHECK-PWR9 %s
+; RUN: llc -verify-machineinstrs -mcpu=pwr8 -mtriple=powerpc64le-unknown-unknown \
+; RUN:   -ppc-asm-full-reg-names < %s | FileCheck -check-prefixes=CHECK-PWR8  %s
+; RUN: llc -verify-machineinstrs -mcpu=pwr7 -mtriple=powerpc64-unknown-unknown \
+; RUN:   -ppc-asm-full-reg-names < %s | FileCheck -check-prefixes=CHECK-PWR7 %s
+
+define <1 x i64> @setcc_v1i128(<1 x i128> %a) {
+; CHECK-PWR9-LABEL: setcc_v1i128:
+; CHECK-PWR9:       # %bb.0: # %entry
+; CHECK-PWR9-NEXT:    mfvsrld r3, vs34
+; CHECK-PWR9-NEXT:    cmpldi r3, 35708
+; CHECK-PWR9-NEXT:    mfvsrd r3, vs34
+; CHECK-PWR9-NEXT:    cmpdi cr1, r3, 0
+; CHECK-PWR9-NEXT:    li r3, 1
+; CHECK-PWR9-NEXT:    crnand 4*cr5+lt, 4*cr1+eq, lt
+; CHECK-PWR9-NEXT:    isel r3, 0, r3, 4*cr5+lt
+; CHECK-PWR9-NEXT:    blr
+;
+; CHECK-PWR8-LABEL: setcc_v1i128:
+; CHECK-PWR8:       # %bb.0: # %entry
+; CHECK-PWR8-NEXT:    xxswapd vs0, vs34
+; CHECK-PWR8-NEXT:    mfvsrd r3, vs34
+; CHECK-PWR8-NEXT:    cmpdi r3, 0
+; CHECK-PWR8-NEXT:    li r3, 1
+; CHECK-PWR8-NEXT:    mffprd r4, f0
+; CHECK-PWR8-NEXT:    cmpldi cr1, r4, 35708
+; CHECK-PWR8-NEXT:    crnand 4*cr5+lt, eq, 4*cr1+lt
+; CHECK-PWR8-NEXT:    isel r3, 0, r3, 4*cr5+lt
+; CHECK-PWR8-NEXT:    blr
+;
+; CHECK-PWR7-LABEL: setcc_v1i128:
+; CHECK-PWR7:       # %bb.0: # %entry
+; CHECK-PWR7-NEXT:    li r5, 0
+; CHECK-PWR7-NEXT:    cntlzd r3, r3
+; CHECK-PWR7-NEXT:    ori r5, r5, 35708
+; CHECK-PWR7-NEXT:    rldicl r3, r3, 58, 63
+; CHECK-PWR7-NEXT:    subc r5, r4, r5
+; CHECK-PWR7-NEXT:    subfe r4, r4, r4
+; CHECK-PWR7-NEXT:    neg r4, r4
+; CHECK-PWR7-NEXT:    and r3, r3, r4
+; CHECK-PWR7-NEXT:    blr
+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
+}
+