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

Fix stack probing for x32 #59674

Closed
crlf0710 opened this issue Apr 3, 2019 · 11 comments · Fixed by #62808
Closed

Fix stack probing for x32 #59674

crlf0710 opened this issue Apr 3, 2019 · 11 comments · Fixed by #62808
Assignees
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-x32 x32 ABI T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@crlf0710
Copy link
Member

crlf0710 commented Apr 3, 2019

I've got a Rust minimal repro code (a.rs), its mir output (a.mir), its x86 codegen output (a.x86.ll) and its gnu-x32 codegen output (a.gnux32.ll) All these files are in this this gist.

Currently, llc a.gnux32.ll fails, saying "llvm error cannot emit physreg copy instruction".
I don't understand llvm ir at all myself, but i tried commenting out L309, L311 & L314 out (all accessing a special 'variable?' called 'unsized_tmp') and llc it passes, then i uncomment L309 and llc fails again.

This issue is blocking #59500. Maybe @eddyb, @oli-obk or someone else who's familiar with rust/src/librustc_codegen_ssa/mir/operand.rs could have a look or give me a hint about this?

@crlf0710
Copy link
Member Author

crlf0710 commented Apr 3, 2019

Forgot to mention: I generated these files with -C panic=abort to make the files shorter, but that's unimportant detail, i guess.

@kennytm kennytm added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-x32 x32 ABI T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 3, 2019
@cramertj
Copy link
Member

cramertj commented Apr 3, 2019

If you haven't yet, you might try using bugpoint to minimize the ll to a smaller reproducer.

@crlf0710
Copy link
Member Author

crlf0710 commented Apr 3, 2019

Using bugpoint, i got this(I used llvm-dis to convert it to text):

; ModuleID = 'bugpoint-reduced-simplified.bc'
source_filename = "b.7rcbfp3g-cgu.0"
target datalayout = "e-m:e-p:32:32-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnux32"

define dso_local void @_ZN1b7call_it17h422679d04fa7b4d4E() unnamed_addr #0 {
start:
  %unsized_tmp = alloca i8, i32 undef, align 16
  unreachable
}

attributes #0 = { "probe-stack"="__rust_probestack" }

!llvm.module.flags = !{!0}

!0 = !{i32 7, !"PIE Level", i32 2}

@luqmana
Copy link
Member

luqmana commented Apr 4, 2019

Looks like it can't deal with the "probe-stack" attribute: removing that makes it work. My guess is the probe stack code doesn't do well with gnux32 targets which is 64bit but has 32bit pointers.

The LLVM debug log re: why it fails:

********** EXPANDING POST-RA PSEUDO INSTRS **********
********** Function: _ZN1b7call_it17h422679d04fa7b4d4E
real copy:   $rax = COPY killed renamable $eax
Cannot copy EAX to RAX
LLVM ERROR: Cannot emit physreg copy instruction

@crlf0710
Copy link
Member Author

crlf0710 commented Apr 4, 2019

Speaking of stack probing, I guess @Zoxc might know more about this?

@sanxiyn
Copy link
Member

sanxiyn commented Apr 4, 2019

To unblock things, I think it is reasonable to delete base.stack_probes = true; line in src/librustc_target/spec/x86_64_unknown_linux_gnux32.rs file while we investigate the issue. Stack probes are x86-only feature that is disabled on other architectures anyway.

@Zoxc
Copy link
Contributor

Zoxc commented Apr 4, 2019

I guess LLVM needs a upstream fix for stack probing on x32 (it probably emits code using 64-bit registers). We may also want a x32 specific stack probe function in https://github.com/rust-lang-nursery/compiler-builtins/blob/master/src/probestack.rs.

@crlf0710
Copy link
Member Author

crlf0710 commented Apr 4, 2019

I've pushed #59686 as the temporary workaround.

@sanxiyn sanxiyn changed the title codegen issue with unsized_locals + gnux32 Fix stack probing for x32 Apr 4, 2019
@crlf0710
Copy link
Member Author

Filed:
LLVM:
https://bugs.llvm.org/show_bug.cgi?id=41477
compiler-builtins:
rust-lang/compiler-builtins#282

@nikic
Copy link
Contributor

nikic commented Apr 20, 2019

Fixed upstream by llvm/llvm-project@b75c8fc. I believe a separate stack probing function for x32 is not necessary, the one for x86_64 should work fine.

@nikic
Copy link
Contributor

nikic commented Jul 18, 2019

The LLVM fix is now in nightly, so we can try re-enabling this again.

Centril added a commit to Centril/rust that referenced this issue Jul 21, 2019
Revert "Disable stack probing for gnux32."

This reverts commit 42d652e. (rust-lang#59686)

Closes rust-lang#59674.
Centril added a commit to Centril/rust that referenced this issue Jul 22, 2019
Revert "Disable stack probing for gnux32."

This reverts commit 42d652e. (rust-lang#59686)

Closes rust-lang#59674.
@bors bors closed this as completed in 66c2965 Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-x32 x32 ABI T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants