llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
26.89k stars 11.04k forks source link

[M68k] RTD must be implemented for 68010 baseline and higher #60554

Open glaubitz opened 1 year ago

glaubitz commented 1 year ago

As initially reported in https://github.com/llvm/llvm-project/issues/59161#issuecomment-1418260415, 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:

glaubitz@node54:/data/home/glaubitz> /data/home/glaubitz/llvm-project/stage1.install/bin/clang -mcpu=68020 -target m68k-linux-gnu future.cc -o future -I /data/home/glaubitz/sid-m68k-sbuild/usr/m68k-linux-gnu/include/c++/12/m68k-linux-gnu/ -I /data/home/glaubitz/sid-m68k-sbuild/usr/m68k-linux-gnu/include/ -I /data/home/glaubitz/sid-m68k-sbuild/usr/include/c++/12/ -I /data/home/glaubitz/sid-m68k-sbuild/usr/include/m68k-linux-gnu/c++/12/ -L /data/home/glaubitz/sid-m68k-sbuild/usr/lib/m68k-linux-gnu/ -L /data/home/glaubitz/sid-m68k-sbuild/usr/lib/gcc/m68k-linux-gnu/12/ -L /data/home/glaubitz/sid-m68k-sbuild/usr/lib/gcc/m68k-linux-gnu/12/ -fuse-ld=/usr/bin/m68k-suse-linux-ld
RTD is not implemented
UNREACHABLE executed at /data/home/glaubitz/llvm-project/llvm/lib/Target/M68k/M68kExpandPseudo.cpp:255!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: /data/home/glaubitz/llvm-project/stage1.install/bin/clang-17 -cc1 -triple m68k-unknown-linux-gnu -emit-obj -mrelax-all -disable-free -clear-ast-before-backend -main-file-name future.cc -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=all -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -target-cpu M68020 -mllvm -treat-scalable-fixed-error-as-warning -debugger-tuning=gdb -fcoverage-compilation-dir=/data/home/glaubitz -resource-dir /data/home/glaubitz/llvm-project/stage1.install/lib/clang/17 -I /data/home/glaubitz/sid-m68k-sbuild/usr/m68k-linux-gnu/include/c++/12/m68k-linux-gnu/ -I /data/home/glaubitz/sid-m68k-sbuild/usr/m68k-linux-gnu/include/ -I /data/home/glaubitz/sid-m68k-sbuild/usr/include/c++/12/ -I /data/home/glaubitz/sid-m68k-sbuild/usr/include/m68k-linux-gnu/c++/12/ -internal-isystem /data/home/glaubitz/llvm-project/stage1.install/lib/clang/17/include -internal-isystem /usr/local/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -fdeprecated-macro -fdebug-compilation-dir=/data/home/glaubitz -ferror-limit 19 -fgnuc-version=4.2.1 -fcxx-exceptions -fexceptions -fcolor-diagnostics -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -o /tmp/future-3f8909.o -x c++ future.cc
1.      <eof> parser at end of file
2.      Code generation
3.      Running pass 'Function Pass Manager' on module 'future.cc'.
4.      Running pass 'M68k pseudo instruction expansion pass' on function '@"_ZSt5asyncIZ4mainE3$_0JEESt6futureINSt15__invoke_resultINSt5decayIT_E4typeEJDpNS3_IT0_E4typeEEE4typeEESt6launchOS4_DpOS7_"'
 #0 0x0000000002003f67 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0x2003f67)
 #1 0x0000000002001a1c SignalHandler(int) Signals.cpp:0:0
 #2 0x00007f95f03a88c0 __restore_rt (/lib64/libpthread.so.0+0x168c0)
 #3 0x00007f95eead8cbb raise (/lib64/libc.so.6+0x4acbb)
 #4 0x00007f95eeada355 abort (/lib64/libc.so.6+0x4c355)
 #5 0x0000000001f6ad7a (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0x1f6ad7a)
 #6 0x0000000000f537bd (anonymous namespace)::M68kExpandPseudo::ExpandMI(llvm::MachineBasicBlock&, llvm::MachineInstrBundleIterator<llvm::MachineInstr, false>) M68kExpandPseudo.cpp:0:0
 #7 0x0000000000f53ab2 (anonymous namespace)::M68kExpandPseudo::runOnMachineFunction(llvm::MachineFunction&) M68kExpandPseudo.cpp:0:0
 #8 0x00000000013633e2 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (.part.71) MachineFunctionPass.cpp:0:0
 #9 0x0000000001900658 llvm::FPPassManager::runOnFunction(llvm::Function&) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0x1900658)
#10 0x0000000001900979 llvm::FPPassManager::runOnModule(llvm::Module&) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0x1900979)
#11 0x00000000019017d8 llvm::legacy::PassManagerImpl::run(llvm::Module&) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0x19017d8)
#12 0x00000000023853a8 clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::StringRef, llvm::Module*, clang::BackendAction, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0x23853a8)
#13 0x00000000030de451 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0x30de451)
#14 0x0000000003dec2d9 clang::ParseAST(clang::Sema&, bool, bool) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0x3dec2d9)
#15 0x00000000030dd0a0 clang::CodeGenAction::ExecuteAction() (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0x30dd0a0)
#16 0x0000000002a847e9 clang::FrontendAction::Execute() (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0x2a847e9)
#17 0x0000000002a1cc3a clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0x2a1cc3a)
#18 0x0000000002b544cb clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0x2b544cb)
#19 0x0000000000bb8ec1 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0xbb8ec1)
#20 0x0000000000bb4398 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) driver.cpp:0:0
#21 0x0000000000bb688c clang_main(int, char**) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang-17+0xbb688c)
#22 0x00007f95eeac329d __libc_start_main (/lib64/libc.so.6+0x3529d)
#23 0x0000000000bae7ba _start /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/../sysdeps/x86_64/start.S:122:0
clang-17: error: unable to execute command: Aborted (core dumped)
clang-17: error: clang frontend command failed due to signal (use -v to see invocation)
clang version 17.0.0 (https://github.com/llvm/llvm-project.git b72330cd54f985f3454d3538b504027c3eff8711)
Target: m68k-unknown-linux-gnu
Thread model: posix
InstalledDir: /data/home/glaubitz/llvm-project/stage1.install/bin
clang-17: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-17: note: diagnostic msg: /tmp/future-76c283.cpp
clang-17: note: diagnostic msg: /tmp/future-76c283.sh
clang-17: note: diagnostic msg: 

********************
glaubitz@node54:/data/home/glaubitz>

The 68020 baseline is required in order to be able to use the newly implemented support for atomics and also the upcoming TLS Support (https://github.com/llvm/llvm-project/issues/60354).

glaubitz commented 1 year ago

CC @0x59616e @mshockwave

llvmbot commented 1 year ago

@llvm/issue-subscribers-backend-m68k

glaubitz commented 1 year ago

@0x59616e Can you implement RTD as a requirement as well? I'm unfortunately not well versed enough yet in IR language to implement it myself.

RKSimon commented 1 year ago

Wasn't the RTD instruction added for the 68010?

glaubitz commented 1 year ago

According to http://68k.hax.com/RTD it requires 68020 or higher.

RKSimon commented 1 year ago

https://gcc.gnu.org/onlinedocs/gcc/M680x0-Options.html suggests 68010

glaubitz commented 1 year ago

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 -mno-mtd flag. That could be added to LLVM as well.

RKSimon commented 1 year ago

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 :)

mshockwave commented 1 year ago

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 -mno-mtd flag. That could be added to LLVM as well.

Just updated the title and description

0x59616e commented 1 year ago

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.

glaubitz commented 1 year ago

Or implement the -mno-rtd flag and hard-wire it to yes for the time being?

mshockwave commented 1 year ago

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.

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?

Or implement the -mno-rtd flag and hard-wire it to yes for the time being?

If we're not going to implement codegen support for RTD then I think -mrtd / -mno-rtd will become an assembler predicate and have no effect on codegen, which I'm fine with.

glaubitz commented 1 year ago

So, I have been digging into this now and what I found is that -mno-rtd is actually already handled by Clang:

def mno_rtd: Flag<["-"], "mno-rtd">, Group<m_Group>;

and

def mno_rtd: Flag<["-"], "mno-rtd">, Group<m_Group>;

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:

https://gcc.gnu.org/onlinedocs/gcc/M680x0-Options.html

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?

mshockwave commented 1 year ago

So, I have been digging into this now and what I found is that -mno-rtd is actually already handled by Clang:

def mno_rtd: Flag<["-"], "mno-rtd">, Group<m_Group>;

and

def mno_rtd: Flag<["-"], "mno-rtd">, Group<m_Group>;

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:

https://gcc.gnu.org/onlinedocs/gcc/M680x0-Options.html

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) I'll cook them into patches.