This is an archive of the discontinued LLVM Phabricator instance.

[M68k] Add new calling convention M68k_RTD
ClosedPublic

Authored by myhsu on May 4 2023, 9:29 AM.

Details

Summary

M68k_RTD is really similar to X86's stdcall, in which callee pops the arguments from the stack. In LLVM IR it can be written as m68k_rtdcc.
This patch also improves how ExpandPseudo Pass handles popping stack at function returns in the absent of the RTD instruction.

Diff Detail

Event Timeline

myhsu created this revision.May 4 2023, 9:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2023, 9:29 AM
myhsu requested review of this revision.May 4 2023, 9:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2023, 9:29 AM
RKSimon added inline comments.May 4 2023, 9:33 AM
llvm/lib/Target/M68k/M68kExpandPseudo.cpp
263

This comment seems to be for code that doesn't currently exist?

myhsu updated this revision to Diff 519662.May 4 2023, 2:32 PM
myhsu marked an inline comment as done.

Update code comments.

llvm/lib/Target/M68k/M68kExpandPseudo.cpp
263

you're right, I'll remove it (and maybe add it back when we have RTD).

0x59616e added inline comments.May 4 2023, 10:29 PM
llvm/lib/Target/M68k/M68kExpandPseudo.cpp
263

I don't see any issue in this comment. Doesn't it illustrate what #265 is doing ? Am I missing something ?

myhsu added inline comments.May 5 2023, 9:17 AM
llvm/lib/Target/M68k/M68kExpandPseudo.cpp
263

The original comment said that code in this block is the fallback implementation in the absent of RTD instruction. The comment only makes sense if we already have RTD and emit RTD in one of the code path here, which is not the case (yet).

myhsu updated this revision to Diff 519981.May 5 2023, 2:52 PM
myhsu edited the summary of this revision. (Show Details)

Introduce m68k_rtdcc the textual LLVM IR designation for M68k_RTD

0x59616e added inline comments.May 5 2023, 7:02 PM
llvm/lib/Target/M68k/M68kExpandPseudo.cpp
263

OhOh I saw the comment you were refering to (which is not the comment I'm thinking about). You are right. No question here ;)

0x59616e added inline comments.May 5 2023, 7:04 PM
llvm/include/llvm/AsmParser/LLToken.h
176 ↗(On Diff #519981)

This is awesome. Thanks !

I didn't see any issue here. Let's wait for the approval from other (more senior) reviewers.

jrtc27 added a subscriber: jrtc27.May 7 2023, 6:43 AM

You need a test for calling a variadic function which, by my reading of GCC's manage and comparing it to X86, should be unaffected

llvm/test/CodeGen/M68k/CConv/rtd-call.ll
2

I don't know if the M68k backend tests do this as religiously as RISCV, but you should probably be using update_mir_test_checks.py for your MIR output tests.

llvm/test/CodeGen/M68k/CConv/rtd-ret.ll
7

nounwind?

myhsu updated this revision to Diff 520738.May 9 2023, 10:03 AM
myhsu marked 2 inline comments as done.
myhsu added a reviewer: jrtc27.
  • Handle vararg calls and returns.
  • Use update_mir_test_checks for MIR tests.
llvm/test/CodeGen/M68k/CConv/rtd-call.ll
2

Thanks, I didn't know there is a UTC for MIR.

0x59616e accepted this revision.May 31 2023, 9:36 PM

I think we're good to go. Thanks !

This revision is now accepted and ready to land.May 31 2023, 9:36 PM
This revision was landed with ongoing or failed builds.Oct 15 2023, 4:15 PM
This revision was automatically updated to reflect the committed changes.