[M68k][GlobalISel] Fix a big-endian-related bug in CallLowering

[0x59616e]

Summary

When dealing with i64 data in function call, such as
incoming argument, return value, passing argument to another
function, GlobalISel splits i64 into 2 i32 in little endian
order. We need to reverse the order since M68k is big endian.

This patch hacks on this by defining our own ValueAssigner.
When spliting arguments, we cache the results. After
the work is done, we store the results back in reverse order.

[0x59616e]

GlobalISel also has the same problem on load / store of i64. We need to hack on Legalizer to make it correct. The next patch will fix this.

[arsenm]

llvm/lib/Target/M68k/GISel/M68kCallLowering.cpp

94–96

I think it would be more appropriate to reverse the order where the parts were split in the first place (i.e. make splitToValueTypes endian aware)

[0x59616e]

llvm/lib/Target/M68k/GISel/M68kCallLowering.cpp

94–96

Hi arsenm:

Thanks for reviewing ! I think you're right. Fixing this in the main framework of CallLowering benefits to all of the big endian user (if any).

I already had some ideas when I hacked on this. I'll put it into practice asap :)

[0x59616e]

I've submitted a new patch D116877 that fixes this in CallLowering directly.

llvm/lib/Target/M68k/GISel/M68kCallLowering.h

CallReturnHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI,
       : M68kIncomingValueHandler(MIRBuilder, MRI), MIB(MIB) {}
 
 private:
   void assignValueToReg(Register ValVReg, Register PhysReg,
                         CCValAssign VA) override;
 
   MachineInstrBuilder &MIB;
 };
+
+struct M68kValueAssigner : public CallLowering::ValueAssigner {
+  M68kValueAssigner(CallingConv::ID CallConv, bool isVarArg,
+                    MachineFunction &MF, LLVMContext &Ctx, bool isIncoming,
+                    CCAssignFn *AssignFn_,
+                    CCAssignFn *AssignFnVarArg_ = nullptr)
+      : ValueAssigner(isIncoming, AssignFn_, AssignFnVarArg_),
+        M68kCCState(CallConv, isVarArg, MF, M68kLocs, Ctx) {}
+
+  bool assignArg(unsigned ValNo, EVT OrigVT, MVT ValVT, MVT LocVT,
+                 CCValAssign::LocInfo LocInfo,
+                 const CallLowering::ArgInfo &Info, ISD::ArgFlagsTy Flags,
+                 CCState &State) override;
+
+  SmallVector<CCValAssign, 2> M68kLocs;
+  CCState M68kCCState;
+};
+
+struct M68kOutgoingValueAssigner : public M68kValueAssigner {
+  M68kOutgoingValueAssigner(CallingConv::ID CallConv, bool isVarArg,
+                            MachineFunction &MF, LLVMContext &Ctx,
+                            CCAssignFn *AssignFn_,
+                            CCAssignFn *AssignFnVarArg_ = nullptr)
+      : M68kValueAssigner(CallConv, isVarArg, MF, Ctx, false, AssignFn_,
+                          AssignFnVarArg_) {}
+};
+
+struct M68kIncomingValueAssigner : public M68kValueAssigner {
+  M68kIncomingValueAssigner(CallingConv::ID CallConv, bool isVarArg,
+                            MachineFunction &MF, LLVMContext &Ctx,
+                            CCAssignFn *AssignFn_,
+                            CCAssignFn *AssignFnVarArg_ = nullptr)
+      : M68kValueAssigner(CallConv, isVarArg, MF, Ctx, true, AssignFn_,
+                          AssignFnVarArg_) {}
+};
+
 } // end namespace llvm
 
 #endif // LLVM_LIB_TARGET_M68K_GLSEL_M68KCALLLOWERING_H

llvm/lib/Target/M68k/GISel/M68kCallLowering.cpp

Register getStackAddress(uint64_t Size, int64_t Offset,
     auto AddrReg = MIRBuilder.buildPtrAdd(p0, SPReg, OffsetReg);
     MPO = MachinePointerInfo::getStack(MIRBuilder.getMF(), Offset);
     return AddrReg.getReg(0);
   }
   MachineInstrBuilder MIB;
   const DataLayout &DL;
   const M68kSubtarget &STI;
 };
+
+// M68k returns i64 value in this way:
+//  63      31      0
+//  |-------|-------|
+//    $d0      $d1
+// but GISel allocates the low bits first
+// so low bits will go to $d0, high bits to $d1.
+// We need to reverse the allocated results.
+
+bool M68kValueAssigner::assignArg(unsigned ValNo, EVT OrigVT, MVT ValVT,
+                                  MVT LocVT, CCValAssign::LocInfo LocInfo,
+                                  const CallLowering::ArgInfo &Info,
+                                  ISD::ArgFlagsTy Flags, CCState &State) {
+  if (Flags.isSplit()) {
+    // We need to update the stack offset.
+    M68kCCState.AllocateStack(State.getNextStackOffset() -
+                                  M68kCCState.getNextStackOffset(),
+                              Align(2));
+  }
+
+  bool result = ValueAssigner::assignArg(
+      ValNo, OrigVT, ValVT, LocVT, LocInfo, Info, Flags,
+      Info.Flags[0].isSplit() ? M68kCCState : State);
+  if (!result && Flags.isSplitEnd()) {
+    for (auto It = M68kLocs.rbegin(); It != M68kLocs.rend(); ++It) {
+      State.addLoc(*It);
+    }
+    M68kLocs.clear();
+    // Update the stack offset back to the original CCState
+    State.AllocateStack(M68kCCState.getNextStackOffset() -
+                            State.getNextStackOffset(),
+                        Align(2));
+  }
+  return result;
+}
+
 bool M68kCallLowering::lowerReturn(MachineIRBuilder &MIRBuilder,
                                    const Value *Val, ArrayRef<Register> VRegs,
                                    FunctionLoweringInfo &FLI,
                                    Register SwiftErrorVReg) const {
 
   auto MIB = MIRBuilder.buildInstrNoInsert(M68k::RTS);
   bool Success = true;
   MachineFunction &MF = MIRBuilder.getMF();
   const Function &F = MF.getFunction();
   MachineRegisterInfo &MRI = MF.getRegInfo();
   const M68kTargetLowering &TLI = *getTLI<M68kTargetLowering>();
   CCAssignFn *AssignFn =
       TLI.getCCAssignFn(F.getCallingConv(), true, F.isVarArg());
   auto &DL = F.getParent()->getDataLayout();
   if (!VRegs.empty()) {
     SmallVector<ArgInfo, 8> SplitArgs;
     ArgInfo OrigArg{VRegs, Val->getType(), 0};
     setArgFlags(OrigArg, AttributeList::ReturnIndex, DL, F);
     splitToValueTypes(OrigArg, SplitArgs, DL, F.getCallingConv());
-    OutgoingValueAssigner ArgAssigner(AssignFn);
+    M68kOutgoingValueAssigner ArgAssigner(F.getCallingConv(), F.isVarArg(), MF,
+                                          F.getContext(), AssignFn);
     M68kOutgoingArgHandler ArgHandler(MIRBuilder, MRI, MIB);
     Success = determineAndHandleAssignments(ArgHandler, ArgAssigner, SplitArgs,
                                             MIRBuilder, F.getCallingConv(),
                                             F.isVarArg());
   }
   MIRBuilder.insertInstr(MIB);
   return Success;
 }
for (const auto &Arg : F.args()) {
     ArgInfo OrigArg{VRegs[I], Arg.getType(), I};
     setArgFlags(OrigArg, I + AttributeList::FirstArgIndex, DL, F);
     splitToValueTypes(OrigArg, SplitArgs, DL, F.getCallingConv());
     ++I;
   }
 
   CCAssignFn *AssignFn =
       TLI.getCCAssignFn(F.getCallingConv(), false, F.isVarArg());
-  IncomingValueAssigner ArgAssigner(AssignFn);
+  M68kIncomingValueAssigner ArgAssigner(F.getCallingConv(), F.isVarArg(), MF,
+                                        F.getContext(), AssignFn);
   FormalArgHandler ArgHandler(MIRBuilder, MRI);
   return determineAndHandleAssignments(ArgHandler, ArgAssigner, SplitArgs,
                                        MIRBuilder, F.getCallingConv(),
                                        F.isVarArg());
 }
 
 void M68kIncomingValueHandler::assignValueToReg(Register ValVReg,
                                                 Register PhysReg,
unsigned Opc = TLI.getTargetMachine().isPositionIndependent() ? M68k::CALLq
                                                                 : M68k::CALLb;
 
   auto MIB = MIRBuilder.buildInstrNoInsert(Opc)
                  .add(Info.Callee)
                  .addRegMask(TRI->getCallPreservedMask(MF, Info.CallConv));
 
   CCAssignFn *AssignFn =
       TLI.getCCAssignFn(F.getCallingConv(), false, F.isVarArg());
-  OutgoingValueAssigner Assigner(AssignFn);
+  M68kOutgoingValueAssigner Assigner(F.getCallingConv(), F.isVarArg(), MF,
+                                     F.getContext(), AssignFn);
   M68kOutgoingArgHandler Handler(MIRBuilder, MRI, MIB);
   if (!determineAndHandleAssignments(Handler, Assigner, OutArgs, MIRBuilder,
                                      Info.CallConv, Info.IsVarArg))
     return false;
 
   if (Info.Callee.isReg())
     constrainOperandRegClass(MF, *TRI, MRI, *STI.getInstrInfo(),
                              *STI.getRegBankInfo(), *MIB, MIB->getDesc(),
                              Info.Callee, 0);
 
   MIRBuilder.insertInstr(MIB);
 
   if (!Info.OrigRet.Ty->isVoidTy()) {
     CCAssignFn *RetAssignFn =
         TLI.getCCAssignFn(F.getCallingConv(), true, F.isVarArg());
 
-    OutgoingValueAssigner Assigner(RetAssignFn, RetAssignFn);
+    M68kOutgoingValueAssigner Assigner(F.getCallingConv(), F.isVarArg(), MF,
+                                       F.getContext(), RetAssignFn);
     CallReturnHandler Handler(MIRBuilder, MRI, MIB);
     if (!determineAndHandleAssignments(Handler, Assigner, InArgs, MIRBuilder,
                                        Info.CallConv, Info.IsVarArg))
       return false;
   }
 
   CallSeqStart.addImm(Assigner.StackOffset).addImm(0);
 
   unsigned AdjStackUp = TII.getCallFrameDestroyOpcode();
   MIRBuilder.buildInstr(AdjStackUp).addImm(Assigner.StackOffset).addImm(0);
 
   return true;
 }
 
 bool M68kCallLowering::enableBigEndian() const { return true; }

llvm/test/CodeGen/M68k/GlobalISel/c-call.ll

 ; CHECK-NEXT:  move.l  %d0, (0,%sp)
 ; CHECK-NEXT:  move.l  %d1, (4,%sp)
 ; CHECK-NEXT:  jsr     callee3
 ; CHECK-NEXT:  adda.l  #12, %sp
 ; CHECK-NEXT:  rts
   %3 = call i32 @callee3(i32 %0, i32 %1)
   ret i32 %3
 }
-declare i32 @callee3(i32, i32)
+declare i32 @callee3(i32, i32)
+
+define i64 @test_ret_i64(i64 %0) nounwind {
+; CHECK-LABEL: test_ret_i64:
+; CHECK:       ; %bb.0
+; CHECK-NEXT:  move.l (8,%sp), %d1
+; CHECK-NEXT:  move.l (4,%sp), %d0
+; CHECK-NEXT:  rts
+  ret i64 %0
+}
+
+define void @test_passing_i64(i64 %0, i64 %1) nounwind {
+; CHECK-LABEL: test_passing_i64:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:  suba.l  #20, %sp
+; CHECK-NEXT:  move.l (28,%sp), %d0
+; CHECK-NEXT:  move.l (24,%sp), %d1
+; CHECK-NEXT:  move.l (36,%sp), %a0
+; CHECK-NEXT:  move.l (32,%sp), %a1
+; CHECK-NEXT:  move.l %a0, (4,%sp)
+; CHECK-NEXT:  move.l %a1, (0,%sp)
+; CHECK-NEXT:  move.l %d0, (12,%sp)
+; CHECK-NEXT:  move.l %d1, (8,%sp)
+; CHECK-NEXT:  jsr callee_test_passing_i64
+; CHECK-NEXT:  adda.l #20, %sp
+; CHECK-NEXT:  rts
+  call void @callee_test_passing_i64(i64 %1, i64 %0)
+  ret void
+}
+declare void @callee_test_passing_i64(i64, i64)

llvm/test/CodeGen/M68k/GlobalISel/irtranslator-ret.ll

define i64 @test_ret3(i64 %a) {
   ; CHECK-LABEL: name: test_ret3
   ; CHECK: bb.1 (%ir-block.0):
   ; CHECK:   [[G_F_I1:%[0-9]+]]:_(p0) = G_FRAME_INDEX
   ; CHECK:   [[G_LOAD1:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I1]](p0)
   ; CHECK:   [[G_F_I2:%[0-9]+]]:_(p0) = G_FRAME_INDEX
   ; CHECK:   [[G_LOAD2:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I2]](p0)
   ; CHECK:   [[G_MERGE_VAL:%[0-9]+]]:_(s64) = G_MERGE_VALUES [[G_LOAD1]](s32), [[G_LOAD2]](s32)
   ; CHECK:   [[G_UNMERGE_VAL1:%[0-9]+]]:_(s32), [[G_UNMERGE_VAL2:%[0-9]+]]:_(s32) = G_UNMERGE_VALUES [[G_MERGE_VAL]](s64)
-  ; CHECK:   $d0 = COPY [[G_UNMERGE_VAL1]](s32)
-  ; CHECK:   $d1 = COPY [[G_UNMERGE_VAL2]](s32)
-  ; CHECK:   RTS implicit $d0, implicit $d1
+  ; CHECK:   $d1 = COPY [[G_UNMERGE_VAL1]](s32)
+  ; CHECK:   $d0 = COPY [[G_UNMERGE_VAL2]](s32)
+  ; CHECK:   RTS implicit $d1, implicit $d0
   ret i64 %a
 }