Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upenable the MSP430 LLVM backend #37672
Conversation
rust-highfive
assigned
alexcrichton
Nov 9, 2016
This comment has been minimized.
This comment has been minimized.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
japaric
reviewed
Nov 9, 2016
| @@ -134,8 +134,9 @@ fn main() { | |||
| version_cmd.arg("--version"); | |||
| let version_output = output(&mut version_cmd); | |||
| let mut parts = version_output.split('.'); | |||
| if let (Some(major), Some(minor)) = (parts.next().and_then(|s| s.parse::<u32>().ok()), | |||
| parts.next().and_then(|s| s.parse::<u32>().ok())) { | |||
| if let (Some(major), Some(minor)) = | |||
This comment has been minimized.
This comment has been minimized.
japaric
Nov 9, 2016
Author
Member
Sorry for the churn. My editor automatically rustfmts files on save
japaric
referenced this pull request
Nov 9, 2016
Closed
(MIR?) ICE: unsupported target word size #37673
brson
added
the
relnotes
label
Nov 9, 2016
This comment has been minimized.
This comment has been minimized.
|
@bors r+ |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
@bors r+ |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Update
|
This comment has been minimized.
This comment has been minimized.
stage1 rustc_llvm went from 40766544 bytes to 40958376. An increase of 187KiB (+0.47%) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Done. Hmm, then how come clang 3.8 can produce object files? It's the object emission logic in clang instead of in llvm?
Can we funnel the bitcode throught some tool (clang?) to get object files? |
This comment has been minimized.
This comment has been minimized.
If it applies cleanly on 3.9.1, you can send it to rust-lang/llvm then we can update the submodule. |
This comment has been minimized.
This comment has been minimized.
Are you sure that it was object file and not bitcode? For example if you try to do this:
It will produce
Well, I can apply it to 3.9 but it will need some work, because some APIs have changed since then. |
This comment has been minimized.
This comment has been minimized.
|
Has anyone tried to update rust to llvm 4? |
This comment has been minimized.
This comment has been minimized.
|
Ubuntu's clang can produce executables though:
And executables are object files. |
This comment has been minimized.
This comment has been minimized.
|
EDIT: typo |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
If it's not trivial to do, you can just wait for an llvm-up |
This comment has been minimized.
This comment has been minimized.
|
I know what's going on now.
I think there is some machinery inside |
This comment has been minimized.
This comment has been minimized.
|
Well, this explains why it never worked for me. I don't have a |
This comment has been minimized.
This comment has been minimized.
I don't have a MSP430 board anymore, so I don't need this fix. But if you are going to test the code on a real hardware be warned that this bug can cause some random crashes. |
This comment has been minimized.
This comment has been minimized.
I can't because I don't have hardware either. But noted. |
This comment has been minimized.
This comment has been minimized.
|
OK.
The logic to do this was shoehorned and it's missing important stuff like proper error reporting but, hey, it sort of works. I've added some notes to the PR description that explain how I tested this (the target specification used, the Rust source file used and how the object files that rustc generated look like) And executables don't look quite right though ... AFAICT, What's pending to do:
I'm going to punt the last two points to someone that actually has the hardware and that has proper knowledge about these microcontrollers. |
This comment has been minimized.
This comment has been minimized.
BTW, this is required to be able to compile |
This comment has been minimized.
This comment has been minimized.
|
|
alexcrichton
reviewed
Nov 10, 2016
|
Nice! Onwards to porting to everything! |
| @@ -358,6 +358,10 @@ pub struct TargetOptions { | |||
| // will 'just work'. | |||
| pub obj_is_bitcode: bool, | |||
|
|
|||
| // LLVM can't produce object files for MSP430. Instead, we'll make LLVM emit | |||
| // assembly and then use `msp430-as` to turn that assembly into an object file | |||
| pub obj_needs_as: bool, | |||
This comment has been minimized.
This comment has been minimized.
alexcrichton
Nov 10, 2016
Member
It looks like we still have the vestigal option -C no_integrated_as, perhaps that could be hooked up to the target specs as well? I believe manually running the assembler is implemented in write::run_assembler
| @@ -557,10 +565,13 @@ unsafe fn optimize_and_codegen(cgcx: &CodegenContext, | |||
| // machine code, instead copy the .o file from the .bc | |||
| let write_bc = config.emit_bc || config.obj_is_bitcode; | |||
| let rm_bc = !config.emit_bc && config.obj_is_bitcode; | |||
| let write_asm = config.emit_asm || config.obj_needs_as; | |||
This comment has been minimized.
This comment has been minimized.
alexcrichton
Nov 10, 2016
Member
I think if we call write::run_assembler or hook into no_integrated_as these changes may not be necessary.
That being said, I'm not sure whether the code path with no_integrated_as still works, I can't imagine anyone's used that in forever...
| // TODO don't hardcode, maybe expose as a `as` field in the target | ||
| // specification | ||
| // TODO how to properly access `sess` here? | ||
| let mut cmd = Command::new("msp430-as"); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
alexcrichton
Nov 10, 2016
Member
(note that this block of additions I think can be subsumed by write::run_assembler)
alexcrichton
added
the
T-tools
label
Nov 10, 2016
This comment has been minimized.
This comment has been minimized.
It does not, AFAIK, except for maybe this MSP430X extension.
No, you don't get any error, but you need to manually specify the linker script.
Yes, and I think it also checks the assembly for some hardware errata, but I'm not sure if it's still relevant. |
This comment has been minimized.
This comment has been minimized.
|
I've looked into the gcc source code:
/* If we have been given a specific MCU name then we may be
able to make use of its hardware multiply capabilities. */
if (msp430_hwmult_type != NONE)
{
if (strcmp ("__mspabi_mpyi", name) == 0)
{
if (msp430_use_f5_series_hwmult ())
name = "__mulhi2_f5";
else if (! msp430_no_hwmult ())
name = "__mulhi2";
}
else if (strcmp ("__mspabi_mpyl", name) == 0)
{
if (msp430_use_f5_series_hwmult ())
name = "__mulsi2_f5";
else if (use_32bit_hwmult ())
name = "__mulsi2_hw32";
else if (! msp430_no_hwmult ())
name = "__mulsi2";
}
}I think llvm always generates |
japaric
added some commits
Nov 9, 2016
japaric
force-pushed the
japaric:msp430
branch
from
d6ae19b
to
a6a2477
Nov 12, 2016
This comment has been minimized.
This comment has been minimized.
|
@alexcrichton Updated the code to use (*) Why the |
japaric
reviewed
Nov 12, 2016
| // LLVM can't produce object files for this target. Instead, we'll make LLVM | ||
| // emit assembly and then use `gcc` to turn that assembly into an object | ||
| // file | ||
| pub no_integrated_as: bool, |
This comment has been minimized.
This comment has been minimized.
japaric
Nov 12, 2016
Author
Member
This ... could have a more descriptive name I supposed. I just use the same name as the -C no-integrated-as flag.
This comment has been minimized.
This comment has been minimized.
|
The cabi stuff is still missing but this is useable already. I'd be nice to have this in a nightly before novemb.rs |
This comment has been minimized.
This comment has been minimized.
|
@japaric |
This comment has been minimized.
This comment has been minimized.
|
Added the cabi stuff but I'd like someone familiar with the LLVM API to review it (@eddyb? With this I can turn the
Using @pftbest Until this hits nightly you can explicitly depend on the standard crates to make Cargo build them: # Cargo.toml
[dependencies]
core = { path = "/path/to/rust/repo/src/libcore } |
This comment has been minimized.
This comment has been minimized.
|
When I do
or {
"arch": "msp430",
"data-layout": "e-m:e-p:16:16-i32:16:32-a:16-n8:16",
"features": "-ext",
"executables": true,
"linker": "msp430-elf-gcc",
"llvm-target": "msp430",
"max-atomic-width": 0,
"no-integrated-as": true,
"os": "none",
"relocation-model": "static",
"target-endian": "little",
"target-pointer-width": "16"
} |
This comment has been minimized.
This comment has been minimized.
Huh? Are you sure you are not looking at an old, previously generated object file? I don't see such thing.
This is You could use a "shim" linker to disable the "extension" stuff on the gcc side:
and change the |
This comment has been minimized.
This comment has been minimized.
|
(Also you should add |
This comment has been minimized.
This comment has been minimized.
|
So, I've added vadzim ~/Downloads/rustmsp $
vadzim ~/Downloads/rustmsp $ ls -l
total 64
-rw-r--r-- 1 vadzim staff 371 Nov 13 20:51 msp430g2553.json
-rw-r--r-- 1 vadzim staff 12506 Dec 17 2015 msp430g2553.ld
-rw-r--r-- 1 vadzim staff 9070 Dec 17 2015 msp430g2553_symbols.ld
vadzim ~/Downloads/rustmsp $ rustc --target=msp430g2553 ../rust/src/libcore/lib.rs --emit=llvm-ir
vadzim ~/Downloads/rustmsp $ ls -l
total 3200
-rw-r--r-- 1 vadzim staff 0 Nov 13 20:51 core.metadata.o
-rw-r--r-- 1 vadzim staff 1604652 Nov 13 20:51 core.o
-rw-r--r-- 1 vadzim staff 371 Nov 13 20:51 msp430g2553.json
-rw-r--r-- 1 vadzim staff 12506 Dec 17 2015 msp430g2553.ld
-rw-r--r-- 1 vadzim staff 9070 Dec 17 2015 msp430g2553_symbols.ld
vadzim ~/Downloads/rustmsp $ file core.o
core.o: ELF 32-bit LSB relocatable, version 1, not stripped
vadzim ~/Downloads/rustmsp $ rustc --version --verbose
rustc 1.15.0-dev (e7cae415e 2016-11-13)
binary: rustc
commit-hash: e7cae415ea207732a199dbdd23fb2baee1946d63
commit-date: 2016-11-13
host: x86_64-apple-darwin
release: 1.15.0-dev
LLVM version: 3.9
vadzim ~/Downloads/rustmsp $I will try to use "shim" linker later. |
This comment has been minimized.
This comment has been minimized.
Fixed this, btw. |
This comment has been minimized.
This comment has been minimized.
Thank you, I saw the commit, but I don't have time to test it. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
|
japaric commentedNov 9, 2016
•
edited
to let people experiment with this target out of tree.
The MSP430 architecture is used in 16-bit microcontrollers commonly used
in Digital Signal Processing applications.
How this was tested:
Declaring a custom target with the following specification:
{ "arch": "msp430", "data-layout": "e-m:e-p:16:16-i32:16:32-a:16-n8:16", "executables": true, "linker": "msp430-elf-gcc", "llvm-target": "msp430", "max-atomic-width": 0, "no-integrated-as": true, "os": "none", "panic-strategy": "abort", "relocation-model": "static", "target-endian": "little", "target-pointer-width": "16" }And this minimal file:
Produces the following object files:
r? @alexcrichton
TODO get this working with Makefiles so nightly releases include this backendTODO measure the increase in binary size+187KiB (+0.47%)FIXME --emit=obj produces empty object files