[MIPS] Fix lowering of irregular vector arguments

[LemonBoy]

Summary

The code deciding how to split the vector in register-sized integers used the integer division operator, thus rounding down the result.
Correct the computation for irregularly-sized types (non-power-of-two, non multiple of 8) by rounding the division result upwards.

[LemonBoy]

Apply clang-format

[LemonBoy]

Apply the TRUNCATE + BITCAST strategy everywhere, at least on MIPS the generated code is noticeably better.

[atanasyan]

I have not checked the patch carefully yet. But it looks like some tests was removed. For example MIPS64R5EL-LABEL: i8_2... Is it intended?

[LemonBoy]

In D98189#2614070, @atanasyan wrote:

I have not checked the patch carefully yet. But it looks like some tests was removed. For example MIPS64R5EL-LABEL: i8_2... Is it intended?

That's update_llc_test_checks.py collapsing tests showing the same output, eg. MIPS64R5 and MIPS64R5EL.

[atanasyan]

I see. Thanks. One more question - is it possible to split the patch: one for "fixing a couple of rounding issues" and another for "Introducing a few more conversion rules"?

[LemonBoy]

Sure. I'll have the first one introduce the SelectionDAG rules and the second one introduce the fixes for the MIPS codegen together with the various tests.

[LemonBoy]

Split this patch, the SelectionDAG part is in D98273

[atanasyan]

LGTM

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

if (PartEVT.getSizeInBits() == ValueVT.getSizeInBits() &&
     return DAG.getNode(ISD::BITCAST, DL, ValueVT, Val);
 
   if (ValueVT.getVectorNumElements() != 1) {
      // Certain ABIs require that vectors are passed as integers. For vectors
      // are the same size, this is an obvious bitcast.
      if (ValueVT.getSizeInBits() == PartEVT.getSizeInBits()) {
        return DAG.getNode(ISD::BITCAST, DL, ValueVT, Val);
      } else if (ValueVT.bitsLT(PartEVT)) {
-       // Bitcast Val back the original type and extract the corresponding
+       uint64_t ScalarSize = ValueVT.getScalarSizeInBits();
+
+       // Val may be wider than the result vector type.
+       if (PartEVT.getSizeInBits() % ScalarSize != 0) {
+         uint64_t ValueSize = ValueVT.getFixedSizeInBits();
+         EVT IntermediateType = EVT::getIntegerVT(*DAG.getContext(), ValueSize);
+         // Drop the extra bits.
+         Val = DAG.getNode(ISD::TRUNCATE, DL, IntermediateType, Val);
+         return DAG.getBitcast(ValueVT, Val);
+       }
+
+       // Bitcast the scalar Val to a vector and extract the corresponding
        // vector we want.
-       unsigned Elts = PartEVT.getSizeInBits() / ValueVT.getScalarSizeInBits();
+       unsigned Elts = PartEVT.getSizeInBits() / ScalarSize;
        EVT WiderVecType = EVT::getVectorVT(*DAG.getContext(),
-                                           ValueVT.getVectorElementType(), Elts);
+                                           ValueVT.getVectorElementType(),
+                                           Elts);
        Val = DAG.getBitcast(WiderVecType, Val);
        return DAG.getNode(ISD::EXTRACT_SUBVECTOR, DL, ValueVT, Val,
                           DAG.getVectorIdxConstant(0, DL));
      }
 
      diagnosePossiblyInvalidConstraint(
          *DAG.getContext(), V, "non-trivial scalar-to-vector conversion");
      return DAG.getUNDEF(ValueVT);
static void getCopyToPartsVector(SelectionDAG &DAG, const SDLoc &DL,
 
   if (IntermediateVT.isVector())
     DestEltCnt = IntermediateVT.getVectorElementCount() * NumIntermediates;
   else
     DestEltCnt = ElementCount::getFixed(NumIntermediates);
 
   EVT BuiltVectorTy = EVT::getVectorVT(
       *DAG.getContext(), IntermediateVT.getScalarType(), DestEltCnt.getValue());
-  if (ValueVT != BuiltVectorTy) {
-    if (SDValue Widened = widenVectorToPartType(DAG, Val, DL, BuiltVectorTy))
-      Val = Widened;
 
+  if (ValueVT == BuiltVectorTy) {
+    // Nothing to do.
+  } else if (ValueVT.getSizeInBits() == BuiltVectorTy.getSizeInBits()) {
+    // Bitconvert vector->vector case.
     Val = DAG.getNode(ISD::BITCAST, DL, BuiltVectorTy, Val);
+  } else if (SDValue Widened = widenVectorToPartType(DAG, Val, DL,
+                                                     BuiltVectorTy)) {
+    Val = Widened;
+  } else if (BuiltVectorTy.getVectorElementType().bitsGE(
+                 ValueVT.getVectorElementType()) &&
+             BuiltVectorTy.getVectorElementCount() ==
+                 ValueVT.getVectorElementCount()) {
+    // Promoted vector extract
+    Val = DAG.getAnyExtOrTrunc(Val, DL, BuiltVectorTy);
   }
 
+  assert(Val.getValueType() == BuiltVectorTy && "Unexpected vector value type");
+
   // Split the vector into intermediate operands.
   SmallVector<SDValue, 8> Ops(NumIntermediates);
   for (unsigned i = 0; i != NumIntermediates; ++i) {
     if (IntermediateVT.isVector()) {
       // This does something sensible for scalable vectors - see the
       // definition of EXTRACT_SUBVECTOR for further details.
       unsigned IntermediateNumElts = IntermediateVT.getVectorMinNumElements();
       Ops[i] =

llvm/lib/Target/Mips/MipsISelLowering.cpp

MVT MipsTargetLowering::getRegisterTypeForCallingConv(LLVMContext &Context,
   return Subtarget.isABI_O32() || VT.getSizeInBits() == 32 ? MVT::i32
                                                            : MVT::i64;
 }
 
 unsigned MipsTargetLowering::getNumRegistersForCallingConv(LLVMContext &Context,
                                                            CallingConv::ID CC,
                                                            EVT VT) const {
   if (VT.isVector())
-    return std::max(((unsigned)VT.getSizeInBits() /
-                     (Subtarget.isABI_O32() ? 32 : 64)),
-                    1U);
+    return divideCeil(VT.getSizeInBits(), Subtarget.isABI_O32() ? 32 : 64);
   return MipsTargetLowering::getNumRegisters(Context, VT);
 }
 
 unsigned MipsTargetLowering::getVectorTypeBreakdownForCallingConv(
     LLVMContext &Context, CallingConv::ID CC, EVT VT, EVT &IntermediateVT,
     unsigned &NumIntermediates, MVT &RegisterVT) const {
   // Break down vector types to either 2 i64s or 4 i32s.
   RegisterVT = getRegisterTypeForCallingConv(Context, CC, VT);
   IntermediateVT = RegisterVT;
   NumIntermediates = VT.getFixedSizeInBits() < RegisterVT.getFixedSizeInBits()
                          ? VT.getVectorNumElements()
-                         : VT.getSizeInBits() / RegisterVT.getSizeInBits();
-
+                         : divideCeil(VT.getSizeInBits(), RegisterVT.getSizeInBits());
   return NumIntermediates;
 }
 
 SDValue MipsTargetLowering::getGlobalReg(SelectionDAG &DAG, EVT Ty) const {
   MachineFunction &MF = DAG.getMachineFunction();
   MipsFunctionInfo *FI = MF.getInfo<MipsFunctionInfo>();
   return DAG.getRegister(FI->getGlobalBaseReg(MF), Ty);
 }

llvm/test/CodeGen/Mips/cconv/vector.ll

 ; MIPS64R5EL-NEXT:    copy_s.d $3, $w2[1]
 ; MIPS64R5EL-NEXT:    jr $ra
 ; MIPS64R5EL-NEXT:    nop
 entry:
   %cond.t = trunc <4 x i32> %cond to <4 x i1>
   %res = select <4 x i1> %cond.t, <4 x float> %arg1, <4 x float> %arg2
   ret <4 x float> %res
 }
+
+; Check that vectors of types with non-power-of-two size are correctly handled.
+
+define <2 x i24> @i24x2(<2 x i24> %a, <2 x i24> %b) {
+; MIPS32-LABEL: i24x2:
+; MIPS32:       # %bb.0: # %Entry
+; MIPS32-NEXT:    addu $2, $4, $6
+; MIPS32-NEXT:    addu $3, $5, $7
+; MIPS32-NEXT:    jr $ra
+; MIPS32-NEXT:    nop
+;
+; MIPS64-LABEL: i24x2:
+; MIPS64:       # %bb.0: # %Entry
+; MIPS64-NEXT:    lui $1, 256
+; MIPS64-NEXT:    daddiu $1, $1, -1
+; MIPS64-NEXT:    dsll $1, $1, 24
+; MIPS64-NEXT:    and $2, $5, $1
+; MIPS64-NEXT:    dsrl $2, $2, 24
+; MIPS64-NEXT:    sll $2, $2, 0
+; MIPS64-NEXT:    and $1, $4, $1
+; MIPS64-NEXT:    dsrl $1, $1, 24
+; MIPS64-NEXT:    sll $1, $1, 0
+; MIPS64-NEXT:    addu $1, $1, $2
+; MIPS64-NEXT:    sll $2, $5, 0
+; MIPS64-NEXT:    sll $3, $4, 0
+; MIPS64-NEXT:    dsll $1, $1, 24
+; MIPS64-NEXT:    addu $2, $3, $2
+; MIPS64-NEXT:    lui $3, 255
+; MIPS64-NEXT:    ori $3, $3, 65535
+; MIPS64-NEXT:    and $2, $2, $3
+; MIPS64-NEXT:    or $2, $2, $1
+; MIPS64-NEXT:    jr $ra
+; MIPS64-NEXT:    nop
+;
+; MIPS32R5EB-LABEL: i24x2:
+; MIPS32R5EB:       # %bb.0: # %Entry
+; MIPS32R5EB-NEXT:    addiu $sp, $sp, -48
+; MIPS32R5EB-NEXT:    .cfi_def_cfa_offset 48
+; MIPS32R5EB-NEXT:    sw $ra, 44($sp) # 4-byte Folded Spill
+; MIPS32R5EB-NEXT:    sw $fp, 40($sp) # 4-byte Folded Spill
+; MIPS32R5EB-NEXT:    .cfi_offset 31, -4
+; MIPS32R5EB-NEXT:    .cfi_offset 30, -8
+; MIPS32R5EB-NEXT:    move $fp, $sp
+; MIPS32R5EB-NEXT:    .cfi_def_cfa_register 30
+; MIPS32R5EB-NEXT:    addiu $1, $zero, -16
+; MIPS32R5EB-NEXT:    and $sp, $sp, $1
+; MIPS32R5EB-NEXT:    sw $7, 28($sp)
+; MIPS32R5EB-NEXT:    sw $6, 20($sp)
+; MIPS32R5EB-NEXT:    sw $5, 12($sp)
+; MIPS32R5EB-NEXT:    sw $4, 4($sp)
+; MIPS32R5EB-NEXT:    ld.d $w0, 16($sp)
+; MIPS32R5EB-NEXT:    ld.d $w1, 0($sp)
+; MIPS32R5EB-NEXT:    addv.d $w0, $w1, $w0
+; MIPS32R5EB-NEXT:    shf.w $w0, $w0, 177
+; MIPS32R5EB-NEXT:    copy_s.w $2, $w0[1]
+; MIPS32R5EB-NEXT:    copy_s.w $3, $w0[3]
+; MIPS32R5EB-NEXT:    move $sp, $fp
+; MIPS32R5EB-NEXT:    lw $fp, 40($sp) # 4-byte Folded Reload
+; MIPS32R5EB-NEXT:    lw $ra, 44($sp) # 4-byte Folded Reload
+; MIPS32R5EB-NEXT:    addiu $sp, $sp, 48
+; MIPS32R5EB-NEXT:    jr $ra
+; MIPS32R5EB-NEXT:    nop
+;
+; MIPS64R5EB-LABEL: i24x2:
+; MIPS64R5EB:       # %bb.0: # %Entry
+; MIPS64R5EB-NEXT:    daddiu $sp, $sp, -32
+; MIPS64R5EB-NEXT:    .cfi_def_cfa_offset 32
+; MIPS64R5EB-NEXT:    dsrl $1, $5, 16
+; MIPS64R5EB-NEXT:    sw $1, 16($sp)
+; MIPS64R5EB-NEXT:    sh $5, 20($sp)
+; MIPS64R5EB-NEXT:    dsrl $1, $4, 16
+; MIPS64R5EB-NEXT:    sw $1, 24($sp)
+; MIPS64R5EB-NEXT:    sh $4, 28($sp)
+; MIPS64R5EB-NEXT:    lb $1, 19($sp)
+; MIPS64R5EB-NEXT:    dsll $1, $1, 8
+; MIPS64R5EB-NEXT:    lbu $2, 20($sp)
+; MIPS64R5EB-NEXT:    or $1, $1, $2
+; MIPS64R5EB-NEXT:    dsll $1, $1, 8
+; MIPS64R5EB-NEXT:    lb $2, 27($sp)
+; MIPS64R5EB-NEXT:    dsll $2, $2, 8
+; MIPS64R5EB-NEXT:    lbu $3, 28($sp)
+; MIPS64R5EB-NEXT:    or $2, $2, $3
+; MIPS64R5EB-NEXT:    lbu $3, 21($sp)
+; MIPS64R5EB-NEXT:    dsll $2, $2, 8
+; MIPS64R5EB-NEXT:    or $1, $3, $1
+; MIPS64R5EB-NEXT:    lh $3, 16($sp)
+; MIPS64R5EB-NEXT:    dsll $3, $3, 8
+; MIPS64R5EB-NEXT:    lbu $4, 18($sp)
+; MIPS64R5EB-NEXT:    or $3, $4, $3
+; MIPS64R5EB-NEXT:    lbu $4, 29($sp)
+; MIPS64R5EB-NEXT:    insert.d $w0[0], $3
+; MIPS64R5EB-NEXT:    insert.d $w0[1], $1
+; MIPS64R5EB-NEXT:    or $1, $4, $2
+; MIPS64R5EB-NEXT:    lh $2, 24($sp)
+; MIPS64R5EB-NEXT:    dsll $2, $2, 8
+; MIPS64R5EB-NEXT:    lbu $3, 26($sp)
+; MIPS64R5EB-NEXT:    or $2, $3, $2
+; MIPS64R5EB-NEXT:    insert.d $w1[0], $2
+; MIPS64R5EB-NEXT:    insert.d $w1[1], $1
+; MIPS64R5EB-NEXT:    addv.d $w0, $w1, $w0
+; MIPS64R5EB-NEXT:    copy_s.d $1, $w0[1]
+; MIPS64R5EB-NEXT:    copy_s.d $2, $w0[0]
+; MIPS64R5EB-NEXT:    sb $2, 10($sp)
+; MIPS64R5EB-NEXT:    dsrl $3, $1, 16
+; MIPS64R5EB-NEXT:    sb $3, 11($sp)
+; MIPS64R5EB-NEXT:    dsrl $2, $2, 8
+; MIPS64R5EB-NEXT:    sh $2, 8($sp)
+; MIPS64R5EB-NEXT:    sb $1, 13($sp)
+; MIPS64R5EB-NEXT:    dsrl $1, $1, 8
+; MIPS64R5EB-NEXT:    sb $1, 12($sp)
+; MIPS64R5EB-NEXT:    lw $1, 8($sp)
+; MIPS64R5EB-NEXT:    dsll $1, $1, 16
+; MIPS64R5EB-NEXT:    lhu $2, 12($sp)
+; MIPS64R5EB-NEXT:    or $2, $2, $1
+; MIPS64R5EB-NEXT:    daddiu $sp, $sp, 32
+; MIPS64R5EB-NEXT:    jr $ra
+; MIPS64R5EB-NEXT:    nop
+;
+; MIPS32R5EL-LABEL: i24x2:
+; MIPS32R5EL:       # %bb.0: # %Entry
+; MIPS32R5EL-NEXT:    addiu $sp, $sp, -48
+; MIPS32R5EL-NEXT:    .cfi_def_cfa_offset 48
+; MIPS32R5EL-NEXT:    sw $ra, 44($sp) # 4-byte Folded Spill
+; MIPS32R5EL-NEXT:    sw $fp, 40($sp) # 4-byte Folded Spill
+; MIPS32R5EL-NEXT:    .cfi_offset 31, -4
+; MIPS32R5EL-NEXT:    .cfi_offset 30, -8
+; MIPS32R5EL-NEXT:    move $fp, $sp
+; MIPS32R5EL-NEXT:    .cfi_def_cfa_register 30
+; MIPS32R5EL-NEXT:    addiu $1, $zero, -16
+; MIPS32R5EL-NEXT:    and $sp, $sp, $1
+; MIPS32R5EL-NEXT:    sw $7, 24($sp)
+; MIPS32R5EL-NEXT:    sw $6, 16($sp)
+; MIPS32R5EL-NEXT:    sw $5, 8($sp)
+; MIPS32R5EL-NEXT:    sw $4, 0($sp)
+; MIPS32R5EL-NEXT:    ld.d $w0, 16($sp)
+; MIPS32R5EL-NEXT:    ld.d $w1, 0($sp)
+; MIPS32R5EL-NEXT:    addv.d $w0, $w1, $w0
+; MIPS32R5EL-NEXT:    copy_s.w $2, $w0[0]
+; MIPS32R5EL-NEXT:    copy_s.w $3, $w0[2]
+; MIPS32R5EL-NEXT:    move $sp, $fp
+; MIPS32R5EL-NEXT:    lw $fp, 40($sp) # 4-byte Folded Reload
+; MIPS32R5EL-NEXT:    lw $ra, 44($sp) # 4-byte Folded Reload
+; MIPS32R5EL-NEXT:    addiu $sp, $sp, 48
+; MIPS32R5EL-NEXT:    jr $ra
+; MIPS32R5EL-NEXT:    nop
+;
+; MIPS64R5EL-LABEL: i24x2:
+; MIPS64R5EL:       # %bb.0: # %Entry
+; MIPS64R5EL-NEXT:    daddiu $sp, $sp, -32
+; MIPS64R5EL-NEXT:    .cfi_def_cfa_offset 32
+; MIPS64R5EL-NEXT:    dsrl $1, $5, 32
+; MIPS64R5EL-NEXT:    sh $1, 20($sp)
+; MIPS64R5EL-NEXT:    sw $5, 16($sp)
+; MIPS64R5EL-NEXT:    dsrl $1, $4, 32
+; MIPS64R5EL-NEXT:    sh $1, 28($sp)
+; MIPS64R5EL-NEXT:    lbu $1, 20($sp)
+; MIPS64R5EL-NEXT:    sw $4, 24($sp)
+; MIPS64R5EL-NEXT:    dsll $1, $1, 8
+; MIPS64R5EL-NEXT:    lbu $2, 19($sp)
+; MIPS64R5EL-NEXT:    or $1, $1, $2
+; MIPS64R5EL-NEXT:    lb $2, 21($sp)
+; MIPS64R5EL-NEXT:    dsll $2, $2, 16
+; MIPS64R5EL-NEXT:    lbu $3, 28($sp)
+; MIPS64R5EL-NEXT:    dsll $3, $3, 8
+; MIPS64R5EL-NEXT:    lb $4, 18($sp)
+; MIPS64R5EL-NEXT:    lbu $5, 27($sp)
+; MIPS64R5EL-NEXT:    or $3, $3, $5
+; MIPS64R5EL-NEXT:    or $1, $1, $2
+; MIPS64R5EL-NEXT:    dsll $2, $4, 16
+; MIPS64R5EL-NEXT:    lhu $4, 16($sp)
+; MIPS64R5EL-NEXT:    or $2, $4, $2
+; MIPS64R5EL-NEXT:    lb $4, 29($sp)
+; MIPS64R5EL-NEXT:    dsll $4, $4, 16
+; MIPS64R5EL-NEXT:    insert.d $w0[0], $2
+; MIPS64R5EL-NEXT:    insert.d $w0[1], $1
+; MIPS64R5EL-NEXT:    or $1, $3, $4
+; MIPS64R5EL-NEXT:    lb $2, 26($sp)
+; MIPS64R5EL-NEXT:    dsll $2, $2, 16
+; MIPS64R5EL-NEXT:    lhu $3, 24($sp)
+; MIPS64R5EL-NEXT:    or $2, $3, $2
+; MIPS64R5EL-NEXT:    insert.d $w1[0], $2
+; MIPS64R5EL-NEXT:    insert.d $w1[1], $1
+; MIPS64R5EL-NEXT:    addv.d $w0, $w1, $w0
+; MIPS64R5EL-NEXT:    copy_s.d $1, $w0[0]
+; MIPS64R5EL-NEXT:    copy_s.d $2, $w0[1]
+; MIPS64R5EL-NEXT:    dsrl $3, $2, 8
+; MIPS64R5EL-NEXT:    sb $3, 12($sp)
+; MIPS64R5EL-NEXT:    dsrl $3, $2, 16
+; MIPS64R5EL-NEXT:    sb $3, 13($sp)
+; MIPS64R5EL-NEXT:    sb $2, 11($sp)
+; MIPS64R5EL-NEXT:    sh $1, 8($sp)
+; MIPS64R5EL-NEXT:    dsrl $1, $1, 16
+; MIPS64R5EL-NEXT:    sb $1, 10($sp)
+; MIPS64R5EL-NEXT:    lh $1, 12($sp)
+; MIPS64R5EL-NEXT:    dsll $1, $1, 32
+; MIPS64R5EL-NEXT:    lwu $2, 8($sp)
+; MIPS64R5EL-NEXT:    or $2, $2, $1
+; MIPS64R5EL-NEXT:    daddiu $sp, $sp, 32
+; MIPS64R5EL-NEXT:    jr $ra
+; MIPS64R5EL-NEXT:    nop
+Entry:
+  %0 = add <2 x i24> %a, %b
+  ret <2 x i24> %0
+}