[SelectionDAG] Legalize vaargs that require vector splitting

[luke]

Summary

This adds vector splitting for vaarg instructions during type legalization

[RKSimon]

Please regenerate the patch diff with context

[luke]

Add full context to the patch

[tpr]

LGTM, but I don't think I know the legalization code well enough to approve this.

[RKSimon]

llvm/test/CodeGen/X86/legalize-vaarg.ll

2

Please regenerate this with update_llc_test_checks.py

[efriedma]

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

1860

This probably doesn't match how the vector will actually be passed on a big-endian target. Endianness doesn't affect the ordering of the elements of a vector in LLVM. (See also http://llvm.org/docs/BigEndianNEON.html .)

[luke]

Run update_llc_test_checks.py

[luke]

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

1860

Ok, so from what I understand then it should be fine to just remove this swap?

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

1860

Yes, but we need test coverage - copying legalize-vaarg.ll as a powerpc test with le and be targets should be straightforward?

[luke]

Removed endianness swapping and added PowerPC BE/LE test: (I'm not familiar with it: does this look ok?)

[RKSimon]

Adding some PowerPC people to double check

llvm/test/CodeGen/PowerPC/legalize-vaarg.ll

46 ↗

(On Diff #200971)

copy and paste?

[luke]

In D60762#1513918, @RKSimon wrote:

Adding some PowerPC people to double check

Thanks!

llvm/test/CodeGen/PowerPC/legalize-vaarg.ll

46 ↗

(On Diff #200971)

Whoops, thanks! I added in this line but looks like update_llc_test_checks.py kept it, will remove

[luke]

Remove stray CHECK

[shchenz]

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

1856

Why vaarg has alignment 0? I think it should be the result of getABITypeAlignment() for type of Hi? and also the alignment for Lo should not be right.

[shchenz]

llvm/test/CodeGen/PowerPC/legalize-vaarg.ll

3 ↗

(On Diff #200974)

triple powerpc64le-unknown-linux-gnu for LE and powerpc64-unknown-linux-gnu for BE?

[luke]

Get alignment from new vector type

[luke]

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

1856

Updated: I presume they should both have the same alignment?

[shchenz]

PowerPC case now LGTM. I think this also needs approval for x86 case?

[RKSimon]

LGTM - thanks

[luke]

@RKSimon @shchenz Thanks. I don't have commit rights, would someone else mind landing this instead?

[RKSimon]

Building this locally with EXPENSIVE_CHECKS enabled causes a large number of warnings in the x86 test:

• • Bad machine code: Missing mayStore flag ***
• function: test_large_vec_vaarg
• basic block: %bb.0 (0x25e70f4da58)
• instruction: %11:gr32 = MOV32rm %stack.0.args, 1, $noreg, 0, $noreg :: (load store 8 on %ir.args)

[luke]

In D60762#1518966, @RKSimon wrote:

Building this locally with EXPENSIVE_CHECKS enabled causes a large number of warnings in the x86 test:

• • Bad machine code: Missing mayStore flag ***
• function: test_large_vec_vaarg
• basic block: %bb.0 (0x25e70f4da58)
• instruction: %11:gr32 = MOV32rm %stack.0.args, 1, $noreg, 0, $noreg :: (load store 8 on %ir.args)

It looks like this happens with all vaarg expansions: The same assertions are thrown without vectors:

define i32 @test_large_vec_vaarg(i32 %n, ...) {
  %args = alloca i8*, align 4
  %x = va_arg i8** %args, i32
  ret i32 %x
}

As well as on the existing vaarg tests (2008-10-29-ExpandVAARG.ll).

[luke]

I've created a separate patch for this assertion

[luke]

@RKSimon the X86 vaarg codegen tests seem to be passing now with EXPENSIVE_CHECKS enabled

[luke]

Rebase against master

[RKSimon]

LGTM - cheers

[luke]

@RKSimon @shchenz I don't have commit access, can someone else commit this for me? Thanks

llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h

private:
   void SplitVecRes_INSERT_VECTOR_ELT(SDNode *N, SDValue &Lo, SDValue &Hi);
   void SplitVecRes_LOAD(LoadSDNode *LD, SDValue &Lo, SDValue &Hi);
   void SplitVecRes_MLOAD(MaskedLoadSDNode *MLD, SDValue &Lo, SDValue &Hi);
   void SplitVecRes_MGATHER(MaskedGatherSDNode *MGT, SDValue &Lo, SDValue &Hi);
   void SplitVecRes_SCALAR_TO_VECTOR(SDNode *N, SDValue &Lo, SDValue &Hi);
   void SplitVecRes_SETCC(SDNode *N, SDValue &Lo, SDValue &Hi);
   void SplitVecRes_VECTOR_SHUFFLE(ShuffleVectorSDNode *N, SDValue &Lo,
                                   SDValue &Hi);
+  void SplitVecRes_VAARG(SDNode *N, SDValue &Lo, SDValue &Hi);
 
   // Vector Operand Splitting: <128 x ty> -> 2 x <64 x ty>.
   bool SplitVectorOperand(SDNode *N, unsigned OpNo);
   SDValue SplitVecOp_VSELECT(SDNode *N, unsigned OpNo);
   SDValue SplitVecOp_VECREDUCE(SDNode *N, unsigned OpNo);
   SDValue SplitVecOp_UnaryOp(SDNode *N);
   SDValue SplitVecOp_TruncateHelper(SDNode *N);
 

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp

case ISD::MGATHER:
     SplitVecRes_MGATHER(cast<MaskedGatherSDNode>(N), Lo, Hi);
     break;
   case ISD::SETCC:
     SplitVecRes_SETCC(N, Lo, Hi);
     break;
   case ISD::VECTOR_SHUFFLE:
     SplitVecRes_VECTOR_SHUFFLE(cast<ShuffleVectorSDNode>(N), Lo, Hi);
     break;
+  case ISD::VAARG:
+    SplitVecRes_VAARG(N, Lo, Hi);
+    break;
 
   case ISD::ANY_EXTEND_VECTOR_INREG:
   case ISD::SIGN_EXTEND_VECTOR_INREG:
   case ISD::ZERO_EXTEND_VECTOR_INREG:
     SplitVecRes_ExtVecInRegOp(N, Lo, Hi);
     break;
 
   case ISD::ABS:
if (useBuildVector) {
       // At least one input vector was used.  Create a new shuffle vector.
       Output =  DAG.getVectorShuffle(NewVT, dl, Op0, Op1, Ops);
     }
 
     Ops.clear();
   }
 }
 
+void DAGTypeLegalizer::SplitVecRes_VAARG(SDNode *N, SDValue &Lo, SDValue &Hi) {
+  EVT OVT = N->getValueType(0);
+  EVT NVT = OVT.getHalfNumVectorElementsVT(*DAG.getContext());
+  SDValue Chain = N->getOperand(0);
+  SDValue Ptr = N->getOperand(1);
+  SDValue SV = N->getOperand(2);
+  SDLoc dl(N);
+  const unsigned Align = N->getConstantOperandVal(3);
+
+  Lo = DAG.getVAArg(NVT, dl, Chain, Ptr, SV, Align);
+  Hi = DAG.getVAArg(NVT, dl, Lo.getValue(1), Ptr, SV, 0);
+  Chain = Hi.getValue(1);
+
+  if (DAG.getDataLayout().isBigEndian())
+    std::swap(Lo, Hi);
+
+  // Modified the chain - switch anything that used the old chain to use
+  // the new one.
+  ReplaceValueWith(SDValue(N, 1), Chain);
+}
+
 
 //===----------------------------------------------------------------------===//
 //  Operand Vector Splitting
 //===----------------------------------------------------------------------===//
 
 /// This method is called when the specified operand of the specified node is
 /// found to need vector splitting. At this point, all of the result types of
 /// the node are known to be legal, but other operands of the node may need

llvm/test/CodeGen/X86/legalize-vaarg.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+;RUN: llc < %s -mtriple=x86_64-- -mattr=avx | FileCheck %s
+
+define <32 x i32> @test_large_vec_vaarg(i32 %n, ...) {
+; CHECK-LABEL: test_large_vec_vaarg:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    movl -{{[0-9]+}}(%rsp), %ecx
+; CHECK-NEXT:    cmpl $24, %ecx
+; CHECK-NEXT:    jae .LBB0_2
+; CHECK-NEXT:  # %bb.1:
+; CHECK-NEXT:    movq {{[0-9]+}}(%rsp), %rax
+; CHECK-NEXT:    addq %rcx, %rax
+; CHECK-NEXT:    addl $8, %ecx
+; CHECK-NEXT:    movl %ecx, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:    jmp .LBB0_3
+; CHECK-NEXT:  .LBB0_2:
+; CHECK-NEXT:    movq (%rsp), %rax
+; CHECK-NEXT:    addq $127, %rax
+; CHECK-NEXT:    andq $-128, %rax
+; CHECK-NEXT:    leaq 32(%rax), %rcx
+; CHECK-NEXT:    movq %rcx, (%rsp)
+; CHECK-NEXT:  .LBB0_3:
+; CHECK-NEXT:    vmovaps (%rax), %ymm0
+; CHECK-NEXT:    movl -{{[0-9]+}}(%rsp), %ecx
+; CHECK-NEXT:    cmpl $24, %ecx
+; CHECK-NEXT:    jae .LBB0_5
+; CHECK-NEXT:  # %bb.4:
+; CHECK-NEXT:    movq {{[0-9]+}}(%rsp), %rax
+; CHECK-NEXT:    addq %rcx, %rax
+; CHECK-NEXT:    addl $8, %ecx
+; CHECK-NEXT:    movl %ecx, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:    jmp .LBB0_6
+; CHECK-NEXT:  .LBB0_5:
+; CHECK-NEXT:    movq (%rsp), %rax
+; CHECK-NEXT:    movq %rax, %rcx
+; CHECK-NEXT:    addq $32, %rcx
+; CHECK-NEXT:    movq %rcx, (%rsp)
+; CHECK-NEXT:  .LBB0_6:
+; CHECK-NEXT:    vmovaps (%rax), %ymm1
+; CHECK-NEXT:    movl -{{[0-9]+}}(%rsp), %ecx
+; CHECK-NEXT:    cmpl $24, %ecx
+; CHECK-NEXT:    jae .LBB0_8
+; CHECK-NEXT:  # %bb.7:
+; CHECK-NEXT:    movq {{[0-9]+}}(%rsp), %rax
+; CHECK-NEXT:    addq %rcx, %rax
+; CHECK-NEXT:    addl $8, %ecx
+; CHECK-NEXT:    movl %ecx, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:    jmp .LBB0_9
+; CHECK-NEXT:  .LBB0_8:
+; CHECK-NEXT:    movq (%rsp), %rax
+; CHECK-NEXT:    movq %rax, %rcx
+; CHECK-NEXT:    addq $32, %rcx
+; CHECK-NEXT:    movq %rcx, (%rsp)
+; CHECK-NEXT:  .LBB0_9:
+; CHECK-NEXT:    vmovaps (%rax), %ymm2
+; CHECK-NEXT:    movl -{{[0-9]+}}(%rsp), %ecx
+; CHECK-NEXT:    cmpl $24, %ecx
+; CHECK-NEXT:    jae .LBB0_11
+; CHECK-NEXT:  # %bb.10:
+; CHECK-NEXT:    movq {{[0-9]+}}(%rsp), %rax
+; CHECK-NEXT:    addq %rcx, %rax
+; CHECK-NEXT:    addl $8, %ecx
+; CHECK-NEXT:    movl %ecx, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:    vmovaps (%rax), %ymm3
+; CHECK-NEXT:    retq
+; CHECK-NEXT:  .LBB0_11:
+; CHECK-NEXT:    movq (%rsp), %rax
+; CHECK-NEXT:    movq %rax, %rcx
+; CHECK-NEXT:    addq $32, %rcx
+; CHECK-NEXT:    movq %rcx, (%rsp)
+; CHECK-NEXT:    vmovaps (%rax), %ymm3
+; CHECK-NEXT:    retq
+  %args = alloca i8*, align 4
+  %x = va_arg i8** %args, <32 x i32>
+  ret <32 x i32> %x
+}