Skip to content
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

error in backend: Cannot emit physreg copy instruction for 32-bit address space #56055

Closed
ararmine opened this issue Jun 15, 2022 · 9 comments · Fixed by #110465
Closed

error in backend: Cannot emit physreg copy instruction for 32-bit address space #56055

ararmine opened this issue Jun 15, 2022 · 9 comments · Fixed by #110465
Labels
backend:X86 llvm:SelectionDAG SelectionDAGISel as well

Comments

@ararmine
Copy link

ararmine commented Jun 15, 2022

The following C++ example using 32-bit address space crashes in back-end:

void f(char * __ptr32 text)
{
    text += 2;
}

Checked with X86-64 clang 14.0.0.
Command to run:

clang test.cpp -S -fms-extensions

Here is the crash:

fatal error: error in backend: Cannot emit physreg copy instruction
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: /opt/compiler-explorer/clang-14.0.0/bin/clang++ -gdwarf-4 -g -o /app/output.s -S --gcc-toolchain=/opt/compiler-explorer/gcc-11.2.0 -fcolor-diagnostics -fno-crash-diagnostics -fms-extensions <source>
1.	<eof> parser at end of file
2.	Code generation
3.	Running pass 'Function Pass Manager' on module '<source>'.
4.	Running pass 'Post-RA pseudo instruction expansion pass' on function '@_Z1fPU10ptr32_sptrc'
 #0 0x00005556c6f7de8f PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
 #1 0x00005556c6f7bd60 llvm::sys::CleanupOnSignal(unsigned long) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x357bd60)
 #2 0x00005556c6eb72d2 llvm::CrashRecoveryContext::HandleExit(int) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x34b72d2)
 #3 0x00005556c6f7492e llvm::sys::Process::Exit(int, bool) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x357492e)
 #4 0x00005556c4b99696 (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x1199696)
 #5 0x00005556c6ebe7ff llvm::report_fatal_error(llvm::Twine const&, bool) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x34be7ff)
 #6 0x00005556c6ebe988 (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x34be988)
 #7 0x00005556c5d1e3c1 llvm::X86InstrInfo::copyPhysReg(llvm::MachineBasicBlock&, llvm::MachineInstrBundleIterator<llvm::MachineInstr, false>, llvm::DebugLoc const&, llvm::MCRegister, llvm::MCRegister, bool) const (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x231e3c1)
 #8 0x00005556c653d3e5 (anonymous namespace)::ExpandPostRA::runOnMachineFunction(llvm::MachineFunction&) ExpandPostRAPseudos.cpp:0:0
 #9 0x00005556c6283638 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x2883638)
#10 0x00005556c66e8ca9 llvm::FPPassManager::runOnFunction(llvm::Function&) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x2ce8ca9)
#11 0x00005556c66e8f41 llvm::FPPassManager::runOnModule(llvm::Module&) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x2ce8f41)
#12 0x00005556c66e99e7 llvm::legacy::PassManagerImpl::run(llvm::Module&) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x2ce99e7)
#13 0x00005556c72ace84 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> >) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x38ace84)
#14 0x00005556c7f0592f clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x450592f)
#15 0x00005556c8d9aa79 clang::ParseAST(clang::Sema&, bool, bool) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x539aa79)
#16 0x00005556c7f05b02 clang::CodeGenAction::ExecuteAction() (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x4505b02)
#17 0x00005556c7901161 clang::FrontendAction::Execute() (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x3f01161)
#18 0x00005556c789c2f2 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x3e9c2f2)
#19 0x00005556c79cc853 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x3fcc853)
#20 0x00005556c4b9b1d4 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x119b1d4)
#21 0x00005556c4b9718d ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) driver.cpp:0:0
#22 0x00005556c77375f5 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
#23 0x00005556c6eb7163 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x34b7163)
#24 0x00005556c77396c8 clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, bool*) const (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x3d396c8)
#25 0x00005556c770cb1a clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&) const (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x3d0cb1a)
#26 0x00005556c770d65f clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*> >&) const (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x3d0d65f)
#27 0x00005556c7716005 clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*> >&) (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x3d16005)
#28 0x00005556c4aa80b2 main (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x10a80b2)
#29 0x00007f83a272c0b3 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b3)
#30 0x00005556c4b96d0a _start (/opt/compiler-explorer/clang-14.0.0/bin/clang+++0x1196d0a)
clang-14: error: clang frontend command failed with exit code 70 (use -v to see invocation)
ASM generation compiler returned: 70
fatal error: error in backend: Cannot emit physreg copy instruction
clang-14: error: clang frontend command failed with exit code 70 (use -v to see invocation)
Execution build compiler returned: 70
@llvmbot
Copy link
Collaborator

llvmbot commented Jun 15, 2022

@llvm/issue-subscribers-backend-x86

@phoebewang
Copy link
Contributor

cc @hvdijk who is familiar with 32-bit address space.

@hvdijk
Copy link
Contributor

hvdijk commented Jun 15, 2022

This isn't exactly the 32-bit address space I'm familiar with, but I'm seeing the last IR before FastISel is

; Function Attrs: mustprogress noinline nounwind optnone uwtable
define dso_local void @_Z1fPU10ptr32_sptrc(ptr addrspace(270) noundef %text) #0 {
entry:
  %text.addr = alloca ptr addrspace(270), align 4
  store ptr addrspace(270) %text, ptr %text.addr, align 4
  %0 = load ptr addrspace(270), ptr %text.addr, align 4
  %add.ptr = getelementptr inbounds i8, ptr addrspace(270) %0, i32 2
  store ptr addrspace(270) %add.ptr, ptr %text.addr, align 4
  ret void
}

which looks valid, and then FastISel produces

bb.0.entry:
  liveins: $edi
  %0:gr32 = COPY $edi
  %1:gr32 = COPY killed %0:gr32
  MOV32mr %stack.0.text.addr, 1, $noreg, 0, $noreg, %1:gr32 :: (store (s32) into %ir.text.addr)
  %6:gr32 = MOV32rm %stack.0.text.addr, 1, $noreg, 0, $noreg :: (load (s32) from %ir.text.addr)
  %5:gr64 = COPY %6:gr32
  %4:gr64 = ADD64ri8 %5:gr64(tied-def 0), 2, implicit-def $eflags
  MOV32mr %stack.0.text.addr, 1, $noreg, 0, $noreg, %4:gr64 :: (store (s32) into %ir.text.addr)
  RET64

where the copy between gr64 and gr32 is not valid.

@hvdijk
Copy link
Contributor

hvdijk commented Jun 15, 2022

This specific problem is in FastISel.cpp and can be fixed by

--- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -549,7 +549,7 @@ bool FastISel::selectGetElementPtr(const User *I) {
   uint64_t TotalOffs = 0;
   // FIXME: What's a good SWAG number for MaxOffs?
   uint64_t MaxOffs = 2048;
-  MVT VT = TLI.getPointerTy(DL);
+  MVT VT = TLI.getPointerTy(DL, I->getType()->getPointerAddressSpace());
   for (gep_type_iterator GTI = gep_type_begin(I), E = gep_type_end(I);
        GTI != E; ++GTI) {
     const Value *Idx = GTI.getOperand();

However, I will leave this for someone more familiar with __ptr32 / __ptr64 to check whether the same problem exists in other places too.

@arsenm
Copy link
Contributor

arsenm commented Jun 15, 2022

getPointerTy should never use the default parameter. This fix is fine in isolation but all other instances should be passed an explicit value

@hvdijk
Copy link
Contributor

hvdijk commented Jun 16, 2022

getPointerTy should never use the default parameter. This fix is fine in isolation but all other instances should be passed an explicit value

Yes, that is marked as a FIXME in the source code already. At some point, we would want to apply

--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -350,15 +350,13 @@ public:
 
   /// Return the pointer type for the given address space, defaults to
   /// the pointer type from the data layout.
-  /// FIXME: The default needs to be removed once all the code is updated.
-  virtual MVT getPointerTy(const DataLayout &DL, uint32_t AS = 0) const {
+  virtual MVT getPointerTy(const DataLayout &DL, uint32_t AS) const {
     return MVT::getIntegerVT(DL.getPointerSizeInBits(AS));
   }
 
   /// Return the in-memory pointer type for the given address space, defaults to
-  /// the pointer type from the data layout.  FIXME: The default needs to be
-  /// removed once all the code is updated.
-  virtual MVT getPointerMemTy(const DataLayout &DL, uint32_t AS = 0) const {
+  /// the pointer type from the data layout.
+  virtual MVT getPointerMemTy(const DataLayout &DL, uint32_t AS) const {
     return MVT::getIntegerVT(DL.getPointerSizeInBits(AS));
   }

However, we still have well over 500 calls that do not pass an explicit address space, and it looks to me like the default address space is the correct address space for the vast majority of them. If we update all of them as part of fixing this bug, the diff becomes so huge as to be un-reviewable.

@ararmine
Copy link
Author

ararmine commented Jun 16, 2022

Thank you for your replies.
I made the same change in my local llvm sources.
I passed the address space value to getPointerTy also in FastISel::getRegForGEPIndex function, it's for the case when getelementptr index type is i64. For this case also FastISel produces invalid copy.

Attaching the source of an example:
example.zip

@ararmine
Copy link
Author

Another example crashing with the same back-end error, this is extracted from AtomicOps test from LLVM test-suite and added ptr32 attribute.

int foo(volatile int * __ptr32 mem, int val, int c) {
  int oldval = __sync_fetch_and_add(mem, val);
  return oldval + c;
}

The invalid COPY instruction is created in InstrEmitter::AddRegisterOperand function. Probably the address space should be taken into account.

@glaubitz
Copy link
Contributor

This problem also shows when trying to build recent versions of the Rust compiler for x32:

   Compiling icu_list v1.4.0
   Compiling rustc_ast_ir v0.0.0 (/home/glaubitz/rust/compiler/rustc_ast_ir)
   Compiling rustc_feature v0.0.0 (/home/glaubitz/rust/compiler/rustc_feature)
   Compiling rustc_ast v0.0.0 (/home/glaubitz/rust/compiler/rustc_ast)
   Compiling rustc_type_ir v0.0.0 (/home/glaubitz/rust/compiler/rustc_type_ir)
   Compiling rustc_baked_icu_data v0.0.0 (/home/glaubitz/rust/compiler/rustc_baked_icu_data)
   Compiling rustc_error_messages v0.0.0 (/home/glaubitz/rust/compiler/rustc_error_messages)
   Compiling gsgdt v0.1.2
rustc-LLVM ERROR: Cannot emit physreg copy instruction
error: could not compile `serde_json` (lib)
warning: build failed, waiting for other jobs to finish...
Build completed unsuccessfully in 0:15:49

arsenm added a commit that referenced this issue Sep 30, 2024
This was using the default address space instead of the
correct one.

Fixes #56055
@arsenm arsenm closed this as completed in 81ba95c Sep 30, 2024
llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Sep 30, 2024
This was using the default address space instead of the
correct one.

Fixes llvm#56055

(cherry picked from commit 81ba95c)
@EugeneZelenko EugeneZelenko added the llvm:SelectionDAG SelectionDAGISel as well label Sep 30, 2024
nikic pushed a commit to llvmbot/llvm-project that referenced this issue Oct 1, 2024
This was using the default address space instead of the
correct one.

Fixes llvm#56055

(cherry picked from commit 81ba95c)
xgupta pushed a commit to xgupta/llvm-project that referenced this issue Oct 4, 2024
This was using the default address space instead of the
correct one.

Fixes llvm#56055
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants