Skip to content

[M68k] Building simple program using std::future results in internal compiler error #59161

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

Closed
glaubitz opened this issue Nov 23, 2022 · 17 comments
Assignees
Labels
backend:m68k crash Prefer [crash-on-valid] or [crash-on-invalid]

Comments

@glaubitz
Copy link
Contributor

glaubitz commented Nov 23, 2022

Trying to cross-compile the following sample program:

#include <thread>
#include <future>
#include <iostream>

int main(){
  auto future = std::async(std::launch::async, [](){
    std::cout << "I'm a thread" << std::endl;
  });

  future.get();

  return 0;
}

Trying to compile this code using the M68k backend results in an internal compiler error:

glaubitz@node54:/data/home/glaubitz> /data/home/glaubitz/llvm-project/stage1.install/bin/clang -target m68k-linux-gnu future.cc -o future.o -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/ -c
Should not custom lower this!
UNREACHABLE executed at /data/home/glaubitz/llvm-project/llvm/lib/Target/M68k/M68kISelLowering.cpp:1353!
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 -target m68k-linux-gnu future.cc -o future.o -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/ -c
1.      <eof> parser at end of file
2.      Code generation
3.      Running pass 'Function Pass Manager' on module 'future.cc'.
4.      Running pass 'M68k DAG->DAG Pattern Instruction Selection' on function '@_ZNSt9once_flag18_Prepare_executionC2IZSt9call_onceIMNSt13__future_base13_State_baseV2EFvPSt8functionIFSt10unique_ptrINS3_12_Result_baseENS7_8_DeleterEEvEEPbEJPS4_SC_SD_EEvRS_OT_DpOT0_EUlvE_EERSI_'
 #0 0x0000000001f5cb9f PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
 #1 0x0000000001f5a754 llvm::sys::CleanupOnSignal(unsigned long) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x1f5a754)
 #2 0x0000000001eab210 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x00007f0e429e48c0 __restore_rt (/lib64/libpthread.so.0+0x168c0)
 #4 0x00007f0e4112acbb raise (/lib64/libc.so.6+0x4acbb)
 #5 0x00007f0e4112c355 abort (/lib64/libc.so.6+0x4c355)
 #6 0x0000000001eb419a (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x1eb419a)
 #7 0x0000000000f002f9 (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0xf002f9)
 #8 0x0000000002dfef40 (anonymous namespace)::SelectionDAGLegalize::LegalizeOp(llvm::SDNode*) (.part.412) LegalizeDAG.cpp:0:0
 #9 0x0000000002e0c598 llvm::SelectionDAG::Legalize() (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x2e0c598)
#10 0x0000000002ecb861 llvm::SelectionDAGISel::CodeGenAndEmitDAG() (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x2ecb861)
#11 0x0000000002ecf22d llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x2ecf22d)
#12 0x0000000002ed0d41 llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) (.part.1088) SelectionDAGISel.cpp:0:0
#13 0x0000000001304db2 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (.part.70) MachineFunctionPass.cpp:0:0
#14 0x000000000185a5c8 llvm::FPPassManager::runOnFunction(llvm::Function&) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x185a5c8)
#15 0x000000000185a8e9 llvm::FPPassManager::runOnModule(llvm::Module&) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x185a8e9)
#16 0x000000000185b748 llvm::legacy::PassManagerImpl::run(llvm::Module&) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x185b748)
#17 0x00000000022c3ef3 clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::StringRef, llvm::Module*, clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x22c3ef3)
#18 0x0000000002fff820 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x2fff820)
#19 0x0000000003ce5ca9 clang::ParseAST(clang::Sema&, bool, bool) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x3ce5ca9)
#20 0x0000000002ffe530 clang::CodeGenAction::ExecuteAction() (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x2ffe530)
#21 0x00000000029c0d19 clang::FrontendAction::Execute() (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x29c0d19)
#22 0x000000000295b23a clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x295b23a)
#23 0x0000000002a8e1fb clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x2a8e1fb)
#24 0x0000000000b30701 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0xb30701)
#25 0x0000000000b2bef8 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) driver.cpp:0:0
#26 0x00000000027ecab5 void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const::'lambda'()>(long) Job.cpp:0:0
#27 0x0000000001eab973 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x1eab973)
#28 0x00000000027efcbe clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x27efcbe)
#29 0x00000000027befd3 clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x27befd3)
#30 0x00000000027bfb83 clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&, bool) const (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x27bfb83)
#31 0x00000000027c610c clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0x27c610c)
#32 0x0000000000b2ea2c clang_main(int, char**) (/data/home/glaubitz/llvm-project/stage1.install/bin/clang+0xb2ea2c)
#33 0x00007f0e4111529d __libc_start_main (/lib64/libc.so.6+0x3529d)
#34 0x0000000000b26d8a _start /home/abuild/rpmbuild/BUILD/glibc-2.31/csu/../sysdeps/x86_64/start.S:122:0
clang-16: error: clang frontend command failed with exit code 134 (use -v to see invocation)
clang version 16.0.0 (https://github.com/llvm/llvm-project.git 907baeec49bfbe9e76498634a9418e1dc6c973d9)
Target: m68k-unknown-linux-gnu
Thread model: posix
InstalledDir: /data/home/glaubitz/llvm-project/stage1.install/bin
clang-16: note: diagnostic msg: 
********************

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

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

Attaching the preprocessed source plus run script in a zipped archive.

future-82834f.zip

CC @mshockwave @0x59616e

@EugeneZelenko EugeneZelenko added backend:m68k crash Prefer [crash-on-valid] or [crash-on-invalid] and removed new issue labels Nov 23, 2022
@llvmbot
Copy link
Member

llvmbot commented Nov 23, 2022

@llvm/issue-subscribers-backend-m68k

@0x59616e 0x59616e self-assigned this Jan 8, 2023
@0x59616e
Copy link
Contributor

0x59616e commented Jan 8, 2023

My initial observation is that we have no support for GlobalTLSAddress. I'm currently looing into this. It will be appreciated if anyone can share your knowledge in the TLS support of M68k.

@glaubitz
Copy link
Contributor Author

glaubitz commented Jan 8, 2023

I can only share that TLS support was lacking for a long time in glibc on m68k and was only added later when ColdFire was developed. I have asked your question on the m68k-specific Debian and Linux kernel mailing lists, there are definitely people who can answer your question(s).

@0x59616e
Copy link
Contributor

0x59616e commented Jan 8, 2023

I can only share that TLS support was lacking for a long time in glibc on m68k and was only added later when ColdFire was developed. I have asked your question on the m68k-specific Debian and Linux kernel mailing lists, there are definitely people who can answer your question(s).

Appreciated ;)

@glaubitz
Copy link
Contributor Author

glaubitz commented Jan 8, 2023

TLS support for m68k was introduced in gcc in gcc-mirror/gcc@75df395.

Maybe this commit can help adding TLS support to LLVM for m68k?

@glaubitz
Copy link
Contributor Author

glaubitz commented Jan 8, 2023

And we don't have TLS support in the backend yet (see: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/M68k/M68kISelDAGToDAG.cpp#L423).

So, I guess adding TLS support will be the next bigger goal for the backend.

@aslak3
Copy link

aslak3 commented Jan 30, 2023

I appreciate that this horse has bolted, but I'd be interested in anyone's thoughts on the performance of the TLS m68k implementation. In general, you see at least a 50% hit, as documented here:

https://news.ycombinator.com/item?id=30007986

And observed on my own 68030 based homebrew board. I'm compelled to run pre TLS (2008-ish) debian, with the latest HEAD kernel, because of the perf hit caused by TLS. The biggest frustration is it impacts non threaded applications (eg, /bin/uname) as well. It's jolly frustrating that this work towards a more complete 68k Linux userland is only really useful for virtualised "machines" (even then the perf hit is the same). Unfortunately I wasn't really following 68k developments back when these choices were made.

Apologies for storming on about something not technically related.

@chewi
Copy link
Contributor

chewi commented Jan 30, 2023

@aslak3 That's okay, I was wondering the same thing. I know there are no free registers, but I was naively wondering whether re-purposing one of them would be less of a performance hit, given that we are looking to break ABI anyway. I only have a vague understanding of assembly though, so I can't offer anything useful here.

@glaubitz
Copy link
Contributor Author

And observed on my own 68030 based homebrew board. I'm compelled to run pre TLS (2008-ish) debian, with the latest HEAD kernel, because of the perf hit caused by TLS. The biggest frustration is it impacts non threaded applications (eg, /bin/uname) as well.

Well, without TLS, we simply won't be able to compile any modern code. So I don't think we really have a choice.

If we're going to be stuck to old code, there is no point in investing into further development, is there?

It's jolly frustrating that this work towards a more complete 68k Linux userland is only really useful for virtualised "machines" (even then the perf hit is the same).

Not really. There is faster hardware available these days such as the Apollo accelerators or the PiStorm.

Yes, it gets slower on older 68k machines, but again, what's would be the point to put more effort into the effort when you can't modern code.

Apologies for storming on about something not technically related.

FWIW, we have a Discord for m68k where these issues can be discussed.

See: https://discord.gg/x4mrPNFy

@glaubitz
Copy link
Contributor Author

@aslak3 That's okay, I was wondering the same thing. I know there are no free registers, but I was naively wondering whether re-purposing one of them would be less of a performance hit, given that we are looking to break ABI anyway. I only have a vague understanding of assembly though, so I can't offer anything useful here.

Well, we broke the ABI on m68k anyway back when TLS support was added. And IIRC, one registers was sacrificed for that matter.

@aslak3
Copy link

aslak3 commented Jan 30, 2023

Well, without TLS, we simply won't be able to compile any modern code. So I don't think we really have a choice.

If we're going to be stuck to old code, there is no point in investing into further development, is there?

My comment was WRT the implementation of TLS support in m68k Linux, not the obvious need for it. It's a syscall overhead for all userspace which is the cause of a significant perf hit, nothing more complex than that. I was wondering what alternative implementations there might have been when it was implemented back in 2008 or so and why they were discounted, etc.

It's jolly frustrating that this work towards a more complete 68k Linux userland is only really useful for virtualised "machines" (even then the perf hit is the same).

Not really. There is faster hardware available these days such as the Apollo accelerators or the PiStorm.

I agree, but also that perf hit applies to all m68k systems equally. For instance, on my AMD64 desktop using the QEMU virt layer I see a 50% hit on disk perf via dd when TLS is in play. I personally find this to be pretty shocking and sad, but that's just my view. As I said, it applies to all software, multithreaded or not.

FWIW, we have a Discord for m68k where these issues can be discussed.

See: https://discord.gg/x4mrPNFy

Yeap I'm on it.

@0x59616e
Copy link
Contributor

0x59616e commented Jan 31, 2023

Seems that the performance issue is stem from the absence of a dedicated thread pointer register (so we are forced to use the __m68k_read_tp )? If so, then there is not much we can do from the compiler's view since it is a runtime issue to a greater or lesser extent.

@glaubitz
Copy link
Contributor Author

glaubitz commented Feb 5, 2023

Coming back to the original issue which is compiling the sample code in future.cc.

With the patches from:

it's possible to successfully compile the code when passing -mcpu=68020 to clang and using the following workaround to disable the emitting of the RTD instruction:

diff --git a/llvm/lib/Target/M68k/M68kExpandPseudo.cpp b/llvm/lib/Target/M68k/M68kExpandPseudo.cpp
index 5383d3107955..8ecd5724354d 100644
--- a/llvm/lib/Target/M68k/M68kExpandPseudo.cpp
+++ b/llvm/lib/Target/M68k/M68kExpandPseudo.cpp
@@ -251,9 +251,9 @@ bool M68kExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
       MIB = BuildMI(MBB, MBBI, DL, TII->get(M68k::RTS));
     } else if (isUInt<16>(StackAdj)) {
 
-      if (STI->atLeastM68020()) {
-        llvm_unreachable("RTD is not implemented");
-      } else {
+      // if (STI->atLeastM68020()) {
+      //   llvm_unreachable("RTD is not implemented");
+      // } else {
         // Copy PC from stack to a free address(A0 or A1) register
         // TODO check if pseudo expand uses free address register
         BuildMI(MBB, MBBI, DL, TII->get(M68k::MOV32aj), M68k::A1)
@@ -269,7 +269,7 @@ bool M68kExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
 
         // RTS
         BuildMI(MBB, MBBI, DL, TII->get(M68k::RTS));
-      }
+       //      }
     } else {
       // TODO: RTD can only handle immediates as big as 2**16-1.
       // If we need to pop off bytes before the return address, we

Thus, in order to completely fix this issue, we will have to implement the RTD instruction in addition to adding TLS support.

@0x59616e
Copy link
Contributor

0x59616e commented Feb 6, 2023

Coming back to the original issue which is compiling the sample code in future.cc.

With the patches from:

it's possible to successfully compile the code when passing -mcpu=68020 to clang and using the following workaround to disable the emitting of the RTD instruction:

diff --git a/llvm/lib/Target/M68k/M68kExpandPseudo.cpp b/llvm/lib/Target/M68k/M68kExpandPseudo.cpp
index 5383d3107955..8ecd5724354d 100644
--- a/llvm/lib/Target/M68k/M68kExpandPseudo.cpp
+++ b/llvm/lib/Target/M68k/M68kExpandPseudo.cpp
@@ -251,9 +251,9 @@ bool M68kExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
       MIB = BuildMI(MBB, MBBI, DL, TII->get(M68k::RTS));
     } else if (isUInt<16>(StackAdj)) {
 
-      if (STI->atLeastM68020()) {
-        llvm_unreachable("RTD is not implemented");
-      } else {
+      // if (STI->atLeastM68020()) {
+      //   llvm_unreachable("RTD is not implemented");
+      // } else {
         // Copy PC from stack to a free address(A0 or A1) register
         // TODO check if pseudo expand uses free address register
         BuildMI(MBB, MBBI, DL, TII->get(M68k::MOV32aj), M68k::A1)
@@ -269,7 +269,7 @@ bool M68kExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
 
         // RTS
         BuildMI(MBB, MBBI, DL, TII->get(M68k::RTS));
-      }
+       //      }
     } else {
       // TODO: RTD can only handle immediates as big as 2**16-1.
       // If we need to pop off bytes before the return address, we

Thus, in order to completely fix this issue, we will have to implement the RTD instruction in addition to adding TLS support.

Glad to hear this ! Could you kindly open an issue for us ? Thanks :)

@glaubitz
Copy link
Contributor Author

glaubitz commented Feb 6, 2023

Thus, in order to completely fix this issue, we will have to implement the RTD instruction in addition to adding TLS support.

Glad to hear this ! Could you kindly open an issue for us ? Thanks :)

Done: #60554

@glaubitz
Copy link
Contributor Author

glaubitz commented Jun 7, 2023

We should be able to close this. There is another minor issue in the backend with regards to TLS though.

Going to open a new issue.

@glaubitz
Copy link
Contributor Author

glaubitz commented Jun 7, 2023

Filed a new issue for the regression I found, see #63162. Closing this one.

@glaubitz glaubitz closed this as completed Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:m68k crash Prefer [crash-on-valid] or [crash-on-invalid]
Projects
None yet
Development

No branches or pull requests

6 participants