[PATCH] D105332: [M68k][GloballSel] Lower outgoing return values in IRTranslator

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu added inline comments.


================
Comment at: llvm/lib/Target/M68k/GlSel/M68kCallLowering.cpp:35
+
+  void assignValueToReg(Register ValVReg, Register PhysReg, CCValAssign &VA) {
+    MIB.addUse(PhysReg, RegState::Implicit);
----------------
myhsu wrote:
> Missing `override` attribute here, which is a compilation error if you build LLVM using Clang.
still outstanding. By saying `override` I mean
```
void assignValueToReg(...) override {
  ...
}
```


================
Comment at: llvm/lib/Target/M68k/GlSel/M68kCallLowering.cpp:42
+  void assignValueToAddress(Register ValVReg, Register Addr, uint64_t Size,
+                            MachinePointerInfo &MPO, CCValAssign &VA) {
+    MachineFunction &MF = MIRBuilder.getMF();
----------------
myhsu wrote:
> myhsu wrote:
> > 1. Missing `override`
> > 2. The method you should override is [[ https://github.com/llvm/llvm-project/blob/bf7f846b683077a8adcb229624082e525870229b/llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h#L260 | 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).
> 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.
ditto missing `override`


================
Comment at: llvm/lib/Target/M68k/GlSel/M68kCallLowering.cpp:50
+  Register getStackAddress(uint64_t Size, int64_t Offset,
+                           MachinePointerInfo &MPO, ISD::ArgFlagsTy Flags) {
+    llvm_unreachable("unimplemented");
----------------
myhsu wrote:
> ditto missing `override`
still outstanding


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105332/new/

https://reviews.llvm.org/D105332