Skip to content

Update version of Rust #78

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
wants to merge 7 commits into from
Closed

Update version of Rust #78

wants to merge 7 commits into from

Conversation

dylanmckay
Copy link
Member

@dylanmckay dylanmckay commented Sep 24, 2017

This PR takes all of the AVR-specific commits from the current avr-support branch and applies them directly to the upstream Rust master.

I've removed all of the "Cherry pick fix for " commits, as they complicate the history and cause merge conflicts easily. I've also cleaned up the history a bit so that there are only a handful of avr-rust specific commits.

I've also ensured that all of our commits are prefixed with "[AVR]", and also added "[No Upstream]" to the relevant commits.

When this is good to go, rather than merging this, we should simply force push avr-support-update to avr-support so that the history is clean.

N.B. The AVR-specific commits are at the top of the branch, but GitHub seems to be showing commits in chronological order rather than showing the commit graph directly.

@dylanmckay
Copy link
Member Author

dylanmckay commented Sep 24, 2017

I now need to update LLVM.

This will be quite straightforward as it looks like the current Rust LLVM version is based on LLVM in Janurary. Looking at the diff between the our current avr-rust/llvm fork, there are a number of changes that we cherry-picked that aren't in the upstream Rust LLVM fork.

This is because the backend is completely in tree now and so the upgrading process is very easy.

Here are a list of all commits that need to be cherry-picked into upstream Rust LLVM

f7a473aae3f [AVR] Implement getCmpLibcallReturnType().
a5e4ec2528e [AVR] Enable the '__do_copy_data' function
26ae3ea19aa (avr-rust/avr-rust-2017-08-24, avr-rust-2017-08-24) [AVR] Use the correct register classes for 16-bit atomic operations
cc4e5d74424 (avr-rust/avr-rust-2017-07-13, avr-rust-2017-07-13) [AVR] Fix indirect calls to function pointers
78a80d531e6 (avr-rust/avr-rust-2017-07-11) [AVR] Use the generic branch relaxer
95ff9f0a762 (avr-rust/avr-rust-2017-07-05, avr-rust/avr-rust-2017-07-04) [AVR] Fix bug which caused assertion errors for some FRMIDX instructions
bd54399d8d6 [AVR] Add a missing clobber declaration to LPMW
f499a1309da (tag: avr-rust-base, avr-rust/avr-rust-2017-06-18) [AVR] Use 1-byte alignment for all types (https://github.com/avr-rust/rust/issues/63 https://github.com/avr-rust/rust/issues/64)
23f0c2e8abc [AVR] When lowering shifts into loops, put newly generated MBBs in the same spot as the original MBB (https://github.com/avr-rust/rust/issues/62)
0917eec4cc1 (avr-rust/avr-rust-2017-05-31) [AVR] Remove SREG from CPI's Uses; authored by Florian Zeitz
0eee50a7533 [AVR] Fix a big in shift operator lowering; Authored by Dr. Gergo Erdi
d80acb0426f [AVR] CPIRdK can only work with r16..r31; Authored by Dr. Gergo Erdi (https://github.com/avr-rust/rust/issues/50)
f3d3277bb71 Merging r298179:
2d52a6e368a (avr-rust/avr-rust-2017-05-13) [AVR] When lowering Select8/Select16, put newly generated MBBs in the same spot
110c6828e08 (gergo/avr-rust-2017-05-03, avr-rust/avr-rust-2017-05-03) [AVR] Reserve the Y register in all functions
06883dd85d0 [AVR] Save/restore the frame pointer for all functions
fc7af3289ce [AVR] Fix a bug where the frame pointer is clobbered
b843dc37fc7 (gergo/avr-rust-2017-05-01, avr-rust/avr-rust-2017-05-01) [AVR] Implement non-constant bit rotations
cbca85fc0e6 (gergo/rust-llvm-2017-04-13-avr, avr-rust/rust-llvm-2017-04-13-avr) [AVR] Fix a bug so that we now emit R_AVR_16 fixups with the correct offset
f2a53d44ce8 [AVR] Implement stacksave/stackrestore by expanding (PR31342)
f2dd609afb6 [AVR] Support zero-sized arguments in defined methods
45b94b35022 [AVR] Do not kill the dest register for a pseudo instruction
243e3dba20f [AVR] Support the LDWRdPtr instruction with the same Src+Dst register

Of course, when Rust upgrades to LLVM 5.0, we can do this process again and ~85% of these commits will already be in upstream Rust

@dylanmckay
Copy link
Member Author

I have pushed a new LLVM branch named avr-rust-experimental

@shepmaster
Copy link
Member

I skimmed through the set of patches that are applied on top of Rust master and they look reasonable. I can try compiling it now.

@shepmaster
Copy link
Member

It does not appear that you have switched to the avr-rust-experimental branch — I've got this change locally:

diff --git a/.gitmodules b/.gitmodules
index 2802c8d639..791c2436cb 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -1,7 +1,7 @@
 [submodule "src/llvm"]
        path = src/llvm
-       url = https://github.com/rust-lang/llvm.git
-       branch = master
+       url = https://github.com/avr-rust/llvm.git
+       branch = avr-rust-experimental
 [submodule "src/rt/hoedown"]
        path = src/rt/hoedown
        url = https://github.com/rust-lang/hoedown.git
diff --git a/src/llvm b/src/llvm
index a6b738c907..ce8e2085be 160000
--- a/src/llvm
+++ b/src/llvm
@@ -1 +1 @@
-Subproject commit a6b738c907ee60e2153f42d8180218bd80b947cc
+Subproject commit ce8e2085bee042b1612dcd99251d46eaeaac831b

@shepmaster
Copy link
Member

I updated libcore mini to this version of Rust and was able to compile and flash my board. I think it's good to go, once you switch to the newest LLVM commit in your branch.

@shepmaster
Copy link
Member

Might be worth updating the layout to e-p:16:8-i8:8-i16:8-i32:8-i64:8-f32:8-f64:8-n8-a:8 as well.

@dylanmckay
Copy link
Member Author

It does not appear that you have switched to the avr-rust-experimental branch

Fixed

Might be worth updating the layout to e-p:16:8-i8:8-i16:8-i32:8-i64:8-f32:8-f64:8-n8-a:8 as well

Fixed

This shouldn't need to be in tree, but I'm committing it anyway because
it's a headache when compiling with a separate LLVM root.
@dylanmckay
Copy link
Member Author

I can't seem to get it to work on my machine

When compiling the avr-rust/blink example, I get this

xargo build --target avr-atmega328p
error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `rustc - --crate-name ___ --print=file-names --crate-type bin --crate-type rlib --target avr-atmega328p` (exit code: 101)
--- stderr
error: Could not create LLVM TargetMachine for triple: avr-unknown-unknown: No available targets are compatible with this triple.

I've tried rebuilding LLVM and Rust from scratch to no avail.

@shepmaster
Copy link
Member

I use my own target spec, maybe something is different?

{
  "llvm-target": "avr-unknown-unknown",
  "cpu": "atmega328p",

  "target-endian": "little",
  "target-pointer-width": "16",
  "os": "none",
  "target-env": "gnu",
  "target-vendor": "unknown",
  "arch": "avr",
  "data-layout": "e-p:16:8-i8:8-i16:8-i32:8-i64:8-f32:8-f64:8-n8-a:8",
  "executables": true,

  "linker": "avr-gcc",
  "linker-flavor": "gcc",
  "pre-link-args": {
    "gcc": [
      "-mmcu=atmega328p",
      "-nostartfiles",
      "../interrupt_vector.S",
      "../initialize_memory.S"
    ]
  },
  "exe-suffix": ".elf",
  "post-link-args": {
    "gcc": [
      "-Wl,--entry=main",
      "-Wl,--entry=_ivr_irq0",
      "-Wl,--entry=_ivr_irq1",
      "-Wl,--entry=_ivr_pin_change_0",
      "-Wl,--entry=_ivr_pin_change_1",
      "-Wl,--entry=_ivr_pin_change_2",
      "-Wl,--entry=_ivr_watchdog_timer",
      "-Wl,--entry=_ivr_timer2_compare_a",
      "-Wl,--entry=_ivr_timer2_compare_b",
      "-Wl,--entry=_ivr_timer2_overflow",
      "-Wl,--entry=_ivr_timer1_capture",
      "-Wl,--entry=_ivr_timer1_compare_a",
      "-Wl,--entry=_ivr_timer1_compare_b",
      "-Wl,--entry=_ivr_timer1_overflow",
      "-Wl,--entry=_ivr_timer0_compare_a",
      "-Wl,--entry=_ivr_timer0_compare_b",
      "-Wl,--entry=_ivr_timer0_overflow",
      "-Wl,--entry=_ivr_spi_transfer_complete",
      "-Wl,--entry=_ivr_usart_rx_complete",
      "-Wl,--entry=_ivr_usart_udr_empty",
      "-Wl,--entry=_ivr_usart_tx_complete",
      "-Wl,--entry=_ivr_adc_conversion_complete",
      "-Wl,--entry=_ivr_eeprom_ready",
      "-Wl,--entry=_ivr_analog_comparator",
      "-Wl,--entry=_ivr_two_wire_serial_interface",
      "-Wl,--entry=_ivr_store_program_memory_ready"
    ]
  },

  "no-compiler-rt": true
}

@shepmaster
Copy link
Member

Hmm. I pulled down avr-rust/blink. After switching to my fork of libcore-mini, I got a different error:

Assertion failed: (InVals.size() == Ins.size() && "LowerFormalArguments didn't emit the correct number of values!"), function LowerArguments, file /Users/shep/Projects/avr-rust/src/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp, line 8141.

@dylanmckay
Copy link
Member Author

I swapped from building an out-of-tree LLVM (using --llvm-root=<path>) to letting Rust build its own LLVM, which fixed the error.

When compiling blink with your new libcore-mini, the compiler itself segfaults

@shepmaster
Copy link
Member

the compiler itself segfaults

Maybe you don't have the LLVM assertions enabled, it's likely to be the error message I pasted earlier?

@dylanmckay
Copy link
Member Author

dylanmckay commented Sep 26, 2017

I get the same error too after I enable assertions

Sounds a lot like #57 eh

@shepmaster
Copy link
Member

Sounds a lot like #57 eh

Indeed; I wonder what's so different about my blink and the official blink...

@shepmaster
Copy link
Member

shepmaster commented Sep 26, 2017

about my blink and the official blink...

Oh, it's probably how we are building, not what:

$ xargo build --target avr-atmega328p
   Compiling blink v0.1.0 (file:///Users/shep/Projects/arduino/official/blink)
Assertion failed: (InVals.size() == Ins.size() && "LowerFormalArguments didn't emit the correct number of values!"), function LowerArguments, file /Users/shep/Projects/avr-rust/src/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp, line 8141.
error: Could not compile `blink`.

To learn more, run the command again with --verbose.

$ xargo build --target avr-atmega328p --release
   Compiling arduino v0.1.0
   Compiling blink v0.1.0 (file:///Users/shep/Projects/arduino/official/blink)
    Finished release [optimized] target(s) in 1.8 secs

When we are not in release, there's probably a lot more pressure on the registers.

@shepmaster
Copy link
Member

how we are building, not what:

Yup, if I build my code in debug mode, then it has the same assertion. My guess is that we haven't actually regressed in any interesting way.

@dylanmckay
Copy link
Member Author

Nice spotting, I think if we edit the usage section in the README to include configure --release, we should be good to go them

@dylanmckay
Copy link
Member Author

Will probably do it in a week or two, have got a lot going on at uni at the moment

If you want to edit the README and land it in the meantime, I've got no problem with that, otherwise it can wait for a bit

@shepmaster
Copy link
Member

I think if we edit the usage section in the README to include configure --release

To be clear, I mean that when compiling your AVR code, not the compiler, you need to use --release

Will probably do it in a week or two

Would you like me to go ahead and perform all the pushes? I can do that 😸

@dylanmckay
Copy link
Member Author

@shepmaster
Copy link
Member

Pushed...!

@shepmaster shepmaster closed this Sep 28, 2017
@shepmaster shepmaster deleted the avr-support-update branch September 28, 2017 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants