-
Notifications
You must be signed in to change notification settings - Fork 12.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[M68k] RTD must be implemented for 68010 baseline and higher #60554
Comments
@llvm/issue-subscribers-backend-m68k |
@0x59616e Can you implement |
Wasn't the RTD instruction added for the 68010? |
According to http://68k.hax.com/RTD it requires 68020 or higher. |
https://gcc.gnu.org/onlinedocs/gcc/M680x0-Options.html suggests 68010 |
You're right. Page 611 of the official processor manual agrees with you. See: https://www.nxp.com/files-static/archives/doc/ref_manual/M68000PRM.pdf And now I learned that gcc has a |
It amazes me how I can remember random things from Amiga programming books over 30 years ago but can't remember what I had for dinner yesterday :) |
Just updated the title and description |
RTD seems to be an optimization rather than a requirement. Can we just remove the check to use RTS for all subtarget and put this on the back burner ? This seems to be not so urgent. |
Or implement the |
slightly tangent to this issue but while I agree with you to use RTS for now, could we at least implement the MC support for RTD -- which is supposed to be easier -- so that people can use it in, say inline assembly?
If we're not going to implement codegen support for RTD then I think |
So, I have been digging into this now and what I found is that
and
Also documented here: https://clang.llvm.org/docs/ClangCommandLineReference.html since it's a standard flag which affects the calling convention. Now, the only thing we need to do is query Clang whether it's supposed to be using the StdCall calling convention or not and default to no which is the same what GCC does: However, I don't see any "UseStdCall()" or similar, so it's unfortunately opaque to me how that works but the fix would be very trivial: index 5383d3107955..004bc90cbebb 100644
--- a/llvm/lib/Target/M68k/M68kExpandPseudo.cpp
+++ b/llvm/lib/Target/M68k/M68kExpandPseudo.cpp
@@ -251,7 +251,7 @@ bool M68kExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
MIB = BuildMI(MBB, MBBI, DL, TII->get(M68k::RTS));
} else if (isUInt<16>(StackAdj)) {
- if (STI->atLeastM68020()) {
+ if (STI->atLeastM68020() && UseStdCall()) {
llvm_unreachable("RTD is not implemented");
} else {
// Copy PC from stack to a free address(A0 or A1) register Does anyone have any idea how to test for StdCall? |
Just a quick update that I've worked out a solution and put at https://github.com/M680x0/M680x0-mono-repo/commits/feature-68k-rtd (the top two commits) |
As initially reported in #59161 (comment), the M68k backend needs to have the RTD instruction implemented in order to use a baseline of 68010 or higher.
The reason for this is that the code in
llvm/lib/Target/M68k/M68kExpandPseudo.cpp
M68kExpandPseudo::ExpandMI()
wants to emit the RTD instead of RTS instruction when the baseline is set to 68010 or higher but RTD is not implemented:The 68020 baseline is required in order to be able to use the newly implemented support for atomics and also the upcoming TLS Support (#60354).
The text was updated successfully, but these errors were encountered: