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.

Sign up for GitHub

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

Jump to bottom

MSP430 compilation fails with "Error: r2 should not be used in indexed addressing mode"; fixed upstream #59077

Closed
cr1901 opened this issue Mar 10, 2019 · 13 comments · Fixed by #62907
Closed

MSP430 compilation fails with "Error: r2 should not be used in indexed addressing mode"; fixed upstream #59077

cr1901 opened this issue Mar 10, 2019 · 13 comments · Fixed by #62907
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.

Comments

@cr1901
Copy link
Contributor

cr1901 commented Mar 10, 2019 •
edited
Loading

Every Rust nightly as of approximately Jan 27, 2019 has failed to build my msp430 application due to assembler errors of the following form:

/home/travis/build/cr1901/AT2XT/target/msp430-none-elf/release/deps/at2xt-37ddc9c42139f11a.at2xt.wity2qy1-cgu.0.rcgu.s:574: Error: r2 should not be used in indexed addressing mode

This is due to a bug in the LLVM backend that has been fixed upstream, but hasn't managed to propagate downstream to rustc yet (thank you @pftbest for the help).

As I understand it, Rust only updates the LLVM backend occassionally, along with some patches; Is it possible to cherry pick this patch so that the msp430 backend works again before the next version bump? I would open this issue on the Rust LLVM fork, but Issues seems to be disabled for this repository...

Meta

$ rustc --version
rustc 1.33.0-nightly (20c2cba61 2019-01-26)

Source
@Mark-Simulacrum
Copy link
Member

I think we're generally speaking tracking trunk quite closely so should be feasible to bump here, cc @alexcrichton

@sanxiyn sanxiyn added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Mar 11, 2019
@cr1901
Copy link
Contributor Author

cr1901 commented Mar 22, 2019

Friendly ping... has there been any update on this?

I'd host my own copy of LLVM with the fix for the time being, but I doubt CI would like that...

@cr1901
Copy link
Contributor Author

cr1901 commented May 7, 2019

@jamesmunns pointed me to this guide. Doesn't seem too difficult to follow; I think I should be able to do this myself, hopefully within the week, but at the very least soon :).

This was referenced May 20, 2019
Merged
Merged
Centril added a commit to Centril/rust that referenced this issue May 21, 2019
@Centril
Rollup merge of rust-lang#60985 - cr1901:msp430-fix, r=alexcrichton
62afa76 
Bump LLVM submodule to fix MSP430 AsmPrinter and assembler syntax mismatch.

Moving on to steps 9 and 10 of the llvm bugfix [guide](https://rust-lang.github.io/rustc-guide/codegen/updating-llvm.html#bugfix-updates), now that Rust's copy of LLVM was [updated](rust-lang/llvm-project#13).

This PR closes issue rust-lang#59077. Nightlies following this PR should have working msp430 codegen again :D.

Thanks for the prompt response even though it took me a while to get this "simple" PR done!
bors added a commit that referenced this issue May 21, 2019
@bors
Auto merge of #60985 - cr1901:msp430-fix, r=alexcrichton
b7f6b8a 
Bump LLVM submodule to fix MSP430 AsmPrinter and assembler syntax mismatch.

Moving on to steps 9 and 10 of the llvm bugfix [guide](https://rust-lang.github.io/rustc-guide/codegen/updating-llvm.html#bugfix-updates), now that Rust's copy of LLVM was [updated](rust-lang/llvm-project#13).

This PR closes issue #59077. Nightlies following this PR should have working msp430 codegen again :D.

Thanks for the prompt response even though it took me a while to get this "simple" PR done!
@mati865
Copy link
Contributor

mati865 commented Jul 19, 2019

Rust now uses LLVM 9, could you try with recent nightly?

@cr1901
Copy link
Contributor Author

cr1901 commented Jul 19, 2019 •
edited
Loading

@mati865 It was fun while it lasted. The transition to LLVM 9 causes a separate error in my AT2XT repo1, so I'm unsure if we've regressed and this particular issue still applies, or if this is simply a totally separate error and the "r2" error was still fixed.

Unfortunately, due to illness, I don't think I can work on this for about another week. Could you ping me again then please if I haven't responded at all. I'll just send another LLVM patch if necessary since I know how to do it now.

  1. (cc: @pftbest Have you seen this? msp430 Rust still shouldn't be using LLVM's assembler for msp430 AFAIK. It may be ready for use however.)

@mati865
Copy link
Contributor

mati865 commented Jul 19, 2019

You can try enabling it here but I have no idea if it works at all:

rust/src/librustc_target/spec/msp430_none_elf.rs

Line 25 in 9ebf478

no_integrated_as: true,

@pftbest
Copy link
Contributor

pftbest commented Jul 23, 2019

@cr1901 I'm looking into this for two evenings already, but still not found where the bug is. What I know so far:

  1. Compiling with --emit=link and --emit=asm both fail, which means it's probably not a no_integrated_as issue, because --emit=asm doesn't need no_integrated_as option.

  2. Compiling with --emit=llvm-ir flag and then using llc to produce the assembly works fine. Which means the bug is specific to how Rust invokes LLVM.

I will keep debugging this and post here if I find something more.

@nikic
Copy link
Contributor

nikic commented Jul 23, 2019

For reference, the error:

LLVM ERROR: Inline asm not supported by this streamer because we don't have an asm parser for this target

Which means that the MSP430 AsmParser cannot be found (even though it does exist). Looking at https://github.com/rust-lang/rust/blob/master/src/librustc_llvm/lib.rs#L74 we see that we indeed fail to initialize it.

@nikic nikic mentioned this issue Jul 23, 2019
Merged
@pftbest
Copy link
Contributor

pftbest commented Jul 23, 2019 •
edited
Loading

@nikic no so long ago the asm parser didn't exist on msp430, the inline assembly was just inserted as is into resulting assembly and everything was fine. What exactly have changed that now the asm parser is required?

P.S. We can also get rid of no_integrated_as option, since we can also produce object files now too.

@nikic
Copy link
Contributor

nikic commented Jul 23, 2019

@pftbest The integrated assembler was enabled in llvm/llvm-project@b26134b, which is why it is now attempted to be used. (The no_integrated_as option in rustc does not control this behavior.)

@pftbest pftbest mentioned this issue Jul 23, 2019
Closed
Centril added a commit to Centril/rust that referenced this issue Jul 24, 2019
@Centril
Rollup merge of rust-lang#62907 - nikic:msp430-asmparser, r=alexcrichton
c9cdfa1 
Initialize the MSP430 AsmParser

Hopefully fixes rust-lang#59077.

r? @alexcrichton
Centril added a commit to Centril/rust that referenced this issue Jul 24, 2019
@Centril
Rollup merge of rust-lang#62907 - nikic:msp430-asmparser, r=alexcrichton
fc18744 
Initialize the MSP430 AsmParser

Hopefully fixes rust-lang#59077.

r? @alexcrichton
Centril added a commit to Centril/rust that referenced this issue Jul 25, 2019
@Centril
Rollup merge of rust-lang#62907 - nikic:msp430-asmparser, r=alexcrichton
7273fa2 
Initialize the MSP430 AsmParser

Hopefully fixes rust-lang#59077.

r? @alexcrichton
Centril added a commit to Centril/rust that referenced this issue Jul 26, 2019
@Centril
Rollup merge of rust-lang#62907 - nikic:msp430-asmparser, r=alexcrichton
51028f9 
Initialize the MSP430 AsmParser

Hopefully fixes rust-lang#59077.

r? @alexcrichton
Centril added a commit to Centril/rust that referenced this issue Jul 26, 2019
@Centril
Rollup merge of rust-lang#62907 - nikic:msp430-asmparser, r=alexcrichton
286be6a 
Initialize the MSP430 AsmParser

Hopefully fixes rust-lang#59077.

r? @alexcrichton
Centril added a commit to Centril/rust that referenced this issue Jul 26, 2019
@Centril
Rollup merge of rust-lang#62907 - nikic:msp430-asmparser, r=alexcrichton
183aab3 
Initialize the MSP430 AsmParser

Hopefully fixes rust-lang#59077.

r? @alexcrichton
@nikic
Copy link
Contributor

nikic commented Jul 26, 2019

Reopening until there is confirmation that this is fixed...

@nikic nikic reopened this Jul 26, 2019
@cr1901
Copy link
Contributor Author

cr1901 commented Jul 26, 2019

@nikic Thank you for your work. There is still a bit more work to be done to get msp430 compiling again. The fix is involved/breaking, and @pftbest and I (mainly me) will not be able to fix this until the week of August 11th.

@pftbest
Copy link
Contributor

pftbest commented Aug 8, 2019

I tested 1.38.0-nightly (ad7c55e 2019-08-07) and can confirm that this issue has been resolved.
@nikic I think you can close this now, thank you.

@nikic nikic closed this as completed Aug 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Initialize the MSP430 AsmParser nikic/rust
6 participants
@sanxiyn @nikic @mati865 @pftbest @Mark-Simulacrum @cr1901