[M68k][GloballSel] Lower outgoing return values in IRTranslator

[sushmaunnibhavi]

Summary

Implementation of lowerReturn in the IRTranslator for the M68k backend.

[gandhi21299]

llvm/lib/Target/M68k/M68kISelLowering.h

177

Since there is the param bool Return, we could combine this function with the above one.

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

185

The name of the register storing the return value, ie $bd0, could be turned into a regex. Please do the same for the tests below.

213

Floating point tests might be doable? In case its a hastle and Minh does not mind, perhaps add a TODO in the IRTranslator somewhere.

[gandhi21299]

@myhsu x64 debian failure is likely because of the missing M68kISelLowering.h. Can we do something about this?

[myhsu]

In D105332#2855581, @gandhi21299 wrote:

@myhsu x64 debian failure is likely because of the missing M68kISelLowering.h. Can we do something about this?

That's really interesting...clang-format always seems to have a hard time finding that file, I'll look into it

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

35

Missing override attribute here, which is a compilation error if you build LLVM using Clang.

42

1. Missing override
2. The method you should override is this one. More specifically, the third argument is mismatched.
3. According to M68k's ABI, we don't use stack for returning, so I don't think we need to implement this method (Put a llvm_unreachable here should be sufficient).

50

ditto missing override

llvm/lib/Target/M68k/M68kISelLowering.h

177

Agree, currently we don't have complicate CC assignment functions so combining them into one is preferred.

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

185

I don't think so...according to M68k ABI it only uses one (and two in some cases) for return values

213

I'm fine with either way. Floating point support is not our main focus now

[myhsu]

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

185

*only uses one (or two) register(s)

[myhsu]

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

42

I just found that the function signature for this method was just changed recently (99c7e918b5ea2262635cc5f80b8887e487227638) so you might use an outdated LLVM tree. Please rebase to the current tip-of-tree.

[sushmaunnibhavi]

Updated patch according to reviewers feedback.

[myhsu]

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

35

still outstanding. By saying override I mean

void assignValueToReg(...) override {
  ...
}

42

ditto missing override

50

still outstanding

[sushmaunnibhavi]

@myhsu does the patch look good?

[myhsu]

LGTM
Thank you!

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

 #include "llvm/CodeGen/GlobalISel/CallLowering.h"
 #include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
 #include "llvm/CodeGen/TargetCallingConv.h"
 
 using namespace llvm;
 
 M68kCallLowering::M68kCallLowering(const M68kTargetLowering &TLI)
     : CallLowering(&TLI) {}
+
+struct OutgoingArgHandler : public CallLowering::OutgoingValueHandler {
+  OutgoingArgHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI,
+                     MachineInstrBuilder MIB)
+      : OutgoingValueHandler(MIRBuilder, MRI), MIB(MIB) {}
+
+  void assignValueToReg(Register ValVReg, Register PhysReg,
+                        CCValAssign &VA) override {
+    MIB.addUse(PhysReg, RegState::Implicit);
+    Register ExtReg = extendRegister(ValVReg, VA);
+    MIRBuilder.buildCopy(PhysReg, ExtReg);
+  }
+
+  void assignValueToAddress(Register ValVReg, Register Addr, LLT MemTy,
+                            MachinePointerInfo &MPO, CCValAssign &VA) override {
+    llvm_unreachable("unimplemented");
+  }
+
+  Register getStackAddress(uint64_t Size, int64_t Offset,
+                           MachinePointerInfo &MPO,
+                           ISD::ArgFlagsTy Flags) override {
+    llvm_unreachable("unimplemented");
+  }
+
+  MachineInstrBuilder MIB;
+};
 bool M68kCallLowering::lowerReturn(MachineIRBuilder &MIRBuilder,
                                    const Value *Val, ArrayRef<Register> VRegs,
                                    FunctionLoweringInfo &FLI,
                                    Register SwiftErrorVReg) const {
 
-  if (Val)
-    return false;
-  MIRBuilder.buildInstr(M68k::RTS);
-  return true;
+  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()};
+    setArgFlags(OrigArg, AttributeList::ReturnIndex, DL, F);
+    splitToValueTypes(OrigArg, SplitArgs, DL, F.getCallingConv());
+    OutgoingValueAssigner ArgAssigner(AssignFn);
+    OutgoingArgHandler ArgHandler(MIRBuilder, MRI, MIB);
+    Success = determineAndHandleAssignments(ArgHandler, ArgAssigner, SplitArgs,
+                                            MIRBuilder, F.getCallingConv(),
+                                            F.isVarArg());
+  }
+  MIRBuilder.insertInstr(MIB);
+  return Success;
 }
 
 bool M68kCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
                                             const Function &F,
                                             ArrayRef<ArrayRef<Register>> VRegs,
                                             FunctionLoweringInfo &FLI) const {
   MachineFunction &MF = MIRBuilder.getMF();
   MachineRegisterInfo &MRI = MF.getRegInfo();
   const auto &DL = F.getParent()->getDataLayout();
   auto &TLI = *getTLI<M68kTargetLowering>();
 
   SmallVector<ArgInfo, 8> SplitArgs;
   unsigned I = 0;
   for (const auto &Arg : F.args()) {
     ArgInfo OrigArg{VRegs[I], Arg.getType()};
     setArgFlags(OrigArg, I + AttributeList::FirstArgIndex, DL, F);
     splitToValueTypes(OrigArg, SplitArgs, DL, F.getCallingConv());
     ++I;
   }
 
   CCAssignFn *AssignFn =
-      TLI.getCCAssignFnForCall(F.getCallingConv(), false, F.isVarArg());
+      TLI.getCCAssignFn(F.getCallingConv(), false, F.isVarArg());
   IncomingValueAssigner ArgAssigner(AssignFn);
   FormalArgHandler ArgHandler(MIRBuilder, MRI);
   return determineAndHandleAssignments(ArgHandler, ArgAssigner, SplitArgs,
                                        MIRBuilder, F.getCallingConv(),
                                        F.isVarArg());
 }
 
 void M68kIncomingValueHandler::assignValueToReg(Register ValVReg,

llvm/lib/Target/M68k/M68kISelLowering.h

public:
   void LowerAsmOperandForConstraint(SDValue Op, std::string &Constraint,
                                     std::vector<SDValue> &Ops,
                                     SelectionDAG &DAG) const override;
 
   MachineBasicBlock *
   EmitInstrWithCustomInserter(MachineInstr &MI,
                               MachineBasicBlock *MBB) const override;
 
-  CCAssignFn *getCCAssignFnForCall(CallingConv::ID CC, bool Return,
-                                   bool IsVarArg) const;
+  CCAssignFn *getCCAssignFn(CallingConv::ID CC, bool Return,
 
 private:
   unsigned GetAlignedArgumentStackSize(unsigned StackSize,
                                        SelectionDAG &DAG) const;
 
   SDValue getReturnAddressFrameIndex(SelectionDAG &DAG) const;
 
   /// Emit a load of return address if tail call
   /// optimization is performed and it is required.
   SDValue EmitTailCallLoadRetAddr(SelectionDAG &DAG, SDValue &OutRetAddr,

llvm/lib/Target/M68k/M68kISelLowering.cpp

case M68kISD::WrapperPC:
     return "M68kISD::WrapperPC";
   case M68kISD::SEG_ALLOCA:
     return "M68kISD::SEG_ALLOCA";
   default:
     return NULL;
   }
 }
 
-CCAssignFn *M68kTargetLowering::getCCAssignFnForCall(CallingConv::ID CC,
-                                                     bool Return,
-                                                     bool IsVarArg) const {
+CCAssignFn *M68kTargetLowering::getCCAssignFn(CallingConv::ID CC, bool Return,
+  if (Return)
+    return RetCC_M68k_C;
+  else
     return CC_M68k_C;
 }

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

define void @test_arg_lowering_struct(%struct.A %a) #0 {
   ; CHECK:   [[G_LOAD3:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I3]](p0)
   ; CHECK:   [[G_F_I4:%[0-9]+]]:_(p0) = G_FRAME_INDEX
   ; CHECK:   [[G_LOAD4:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I4]](p0)
   ; CHECK:   [[G_F_I5:%[0-9]+]]:_(p0) = G_FRAME_INDEX
   ; CHECK:   [[G_LOAD5:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I5]](p0)
   ; CHECK:   RTS
   ret void
 }
+
+define i8 @test_ret1(i8 %a) {
+  ; CHECK-LABEL: name: test_ret1
+  ; 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_TRUNC1:%[0-9]+]]:_(s8) = G_TRUNC [[G_LOAD1]](s32)
+  ; CHECK:   $bd0 = COPY [[G_TRUNC1]](s8)
+  ; CHECK:   RTS implicit $bd0
+  ret i8 %a
+}
+
+define i32 @test_ret2(i32 %a) {
+  ; CHECK-LABEL: name: test_ret2
+  ; 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:   $d0 = COPY [[G_LOAD1]](s32)
+  ; CHECK:   RTS implicit $d0
+  ret i32 %a
+}
+
+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
+  ret i64 %a
+}