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

Enable thumbv7a-pc-windows-msvc target build end to end in rust/master #60895

Merged
merged 4 commits into from
May 20, 2019
Merged

Enable thumbv7a-pc-windows-msvc target build end to end in rust/master #60895

merged 4 commits into from
May 20, 2019

Conversation

chandde
Copy link
Contributor

@chandde chandde commented May 16, 2019

With this PR, plus another commit rust-lang/compiler-builtins@cf98161, I'm able to build the target thumbv7a-pc-windows-msvc successfully, and I'm able to use the stage2 artifacts to build arm32 projects. The commit in compiler_builtins is in release 0.1.14, the current cargo.lock in rust master still uses 0.1.12, so I bumped the compiler_builtins version in cargo.lock to 0.1.15

The command I used to build rust

c:\python27\python.exe x.py build --host x86_64-pc-windows-msvc --build x86_64-pc-windows-msvc --target thumbv7a-pc-windows-msvc --verbose

Changes

  1. update cargolock to use compiler_builtins 0.1.15
  2. handle libunwind in libtest for thumv7a the same as what we have for aarch64
  3. in llvm codegen add a field in CodegenContext to carry the arch, so later in create_msvc_imps function, the arch can be used to check against "x86", instead of 32 pointer width. Apparently Thumv7a is handled differently than x86.

Background
I'm from Microsoft working on enabling Azure IoTEdge on ARM32 Windows IoTCore, Azure IoTEdge has a component called IoTEdged written in rust as a NT service running on Windows, so we need to enable rust on thumbv7a in order to have full IoTEdge. My colleague had made some heavy lifting and we've been using our private toolchain to build IoTEdged in our devops pipeline, because at that time we cannot build thumbv7a target end to end successfully. This change is a followup to enable the end to end build for thumbv7a-pc-windows-msvc target.

Next step
I'll submit more changes to have this target built nightly in rust/master, to achieve the same availability for aarch64-pc-windows-msvc, indexed here https://rust-lang.github.io/rustup-components-history/aarch64-pc-windows-msvc.html and can be manually installed. Please do share what takes to make this happen, is there a formal process I need to follow?

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (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.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 16, 2019
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:0ab03768:start=1558047789072367083,finish=1558047876315381796,duration=87243014713
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
[00:02:16]    Compiling toml v0.4.10
[00:02:16]    Compiling serde_json v1.0.33
[00:02:19]    Compiling bootstrap v0.0.0 (/checkout/src/bootstrap)
[00:02:53]     Finished dev [unoptimized] target(s) in 1m 25s
[00:02:53] error: failed to write /checkout/Cargo.lock
[00:02:53] Caused by:
[00:02:53]   failed to open: /checkout/Cargo.lock
[00:02:53] 
[00:02:53] Caused by:
---
[00:02:53] Makefile:69: recipe for target 'prepare' failed
[00:02:53] make: *** [prepare] Error 1
[00:02:54] Command failed. Attempt 2/5:
[00:02:55]     Finished dev [unoptimized] target(s) in 0.39s
[00:02:55] error: failed to write /checkout/Cargo.lock
[00:02:55] Caused by:
[00:02:55]   failed to open: /checkout/Cargo.lock
[00:02:55] 
[00:02:55] Caused by:
---
[00:02:55] make: *** [prepare] Error 1
[00:02:55] Makefile:69: recipe for target 'prepare' failed
[00:02:57] Command failed. Attempt 3/5:
[00:02:57]     Finished dev [unoptimized] target(s) in 0.38s
[00:02:58] error: failed to write /checkout/Cargo.lock
[00:02:58] Caused by:
[00:02:58]   failed to open: /checkout/Cargo.lock
[00:02:58] 
[00:02:58] Caused by:
---
[00:02:58] make: *** [prepare] Error 1
[00:02:58] Makefile:69: recipe for target 'prepare' failed
[00:03:01] Command failed. Attempt 4/5:
[00:03:01]     Finished dev [unoptimized] target(s) in 0.39s
[00:03:02] error: failed to write /checkout/Cargo.lock
[00:03:02] Caused by:
[00:03:02]   failed to open: /checkout/Cargo.lock
[00:03:02] 
[00:03:02] Caused by:
---
[00:03:02] Makefile:69: recipe for target 'prepare' failed
[00:03:02] make: *** [prepare] Error 1
[00:03:06] Command failed. Attempt 5/5:
[00:03:06]     Finished dev [unoptimized] target(s) in 0.39s
[00:03:07] error: failed to write /checkout/Cargo.lock
[00:03:07] Caused by:
[00:03:07]   failed to open: /checkout/Cargo.lock
[00:03:07] 
[00:03:07] Caused by:

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@chandde
Copy link
Contributor Author

chandde commented May 16, 2019

Undo the changes to bump compiler_builtins to hopefully have a good build. The version bump perhaps is worth another separate PR, @alexcrichton ?

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:2b706400:start=1558050274935843013,finish=1558050362613859735,duration=87678016722
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---

[00:03:54] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:55] tidy error: /checkout/src/libtest/lib.rs:40: line longer than 100 chars
[00:04:00] some tidy checks failed
[00:04:00] 
[00:04:00] 
[00:04:00] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:00] 
[00:04:00] 
[00:04:00] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:00] Build completed unsuccessfully in 0:01:14
[00:04:00] Build completed unsuccessfully in 0:01:14
[00:04:00] Makefile:67: recipe for target 'tidy' failed
[00:04:00] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:04724862
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Thu May 16 23:50:12 UTC 2019
---
travis_time:end:013a9478:start=1558050613002999815,finish=1558050613007613562,duration=4613747
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0214fd96
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:07d4c2a8
travis_time:start:07d4c2a8
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:188c2180
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@mzji
Copy link

mzji commented May 17, 2019

tidy check
[00:03:55] tidy error: /checkout/src/libtest/lib.rs:40: line longer than 100 chars
[00:04:00] some tidy checks failed
[00:04:00] 

@chandde I think you should wrap this line to not exceed 80 columns.

@mati865
Copy link
Contributor

mati865 commented May 17, 2019

@chandde compiler-builtins are handled in #60841 you should probably remove both update and revert commits.

@alexcrichton
Copy link
Member

r=me with tidy passing, thanks @chandde!

src/libtest/lib.rs Outdated Show resolved Hide resolved
@chandde
Copy link
Contributor Author

chandde commented May 17, 2019

thanks for the comments, I'm out attending some school activities with my son and won't have access to a pc until tonight. Will address all the comments later and get back.

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:0738e0f4:start=1558150358557395921,finish=1558150446393389058,duration=87835993137
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---

[00:03:57] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:58] tidy error: /checkout/src/libtest/lib.rs:40: line longer than 100 chars
[00:04:03] some tidy checks failed
[00:04:03] 
[00:04:03] 
[00:04:03] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:03] 
[00:04:03] 
[00:04:03] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:03] Build completed unsuccessfully in 0:01:13
[00:04:03] Build completed unsuccessfully in 0:01:13
[00:04:03] make: *** [tidy] Error 1
[00:04:03] Makefile:67: recipe for target 'tidy' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0887a5ea
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Sat May 18 03:38:19 UTC 2019
---
travis_time:end:1c69de92:start=1558150699938187985,finish=1558150699943265664,duration=5077679
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:00225c1e
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:00668f5d
travis_time:start:00668f5d
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0a090fcd
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@chandde
Copy link
Contributor Author

chandde commented May 18, 2019

@mati865 the update and revert have been forcefully removed in the commit history
@mzji @mati865 the line in libtest\lib.rs has been updated to #[cfg(not(all(windows, any(target_arch = "aarch64", target_arch = "arm"))))], should have fixed the test failure and also look simpler

@mzji
Copy link

mzji commented May 18, 2019

@mati865 the update and revert have been forcefully removed in the commit history
@mzji @mati865 the line in libtest\lib.rs has been updated to #[cfg(not(all(windows, any(target_arch = "aarch64", target_arch = "arm"))))], should have fixed the test failure and also look simpler

No, the tidy error line is the 40th line of src/libtest/lib.rs, looks like you didn't change it

@chandde
Copy link
Contributor Author

chandde commented May 18, 2019

@mati865 the update and revert have been forcefully removed in the commit history
@mzji @mati865 the line in libtest\lib.rs has been updated to #[cfg(not(all(windows, any(target_arch = "aarch64", target_arch = "arm"))))], should have fixed the test failure and also look simpler

No, the tidy error line is the 40th line of src/libtest/lib.rs, looks like you didn't change it

Now it's done done 759921e

I misread the comment and thought it's about the cfg line

@chandde
Copy link
Contributor Author

chandde commented May 19, 2019

r=me with tidy passing, thanks @chandde!

I don't understand the codes, did you say you're ok with the change once tidy passing? who's gonna sign off & merge? @alexcrichton

@jonas-schievink
Copy link
Contributor

@chandde that's Mozilla "slang" that means your PR has been reviewed by whoever comments this and is ready to land when the remaining issues are addressed (in this case, the line length check).

Since that has been addressed:

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented May 19, 2019

📌 Commit 759921e has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 19, 2019
Centril added a commit to Centril/rust that referenced this pull request May 19, 2019
Enable thumbv7a-pc-windows-msvc target build end to end in rust/master

With this PR, plus another commit rust-lang/compiler-builtins@cf98161, I'm able to build the target thumbv7a-pc-windows-msvc successfully, and I'm able to use the stage2 artifacts to build arm32 projects. The commit in compiler_builtins is in release 0.1.14, the current cargo.lock in rust master still uses 0.1.12, so I bumped the compiler_builtins version in cargo.lock to 0.1.15

The command I used to build rust
```
c:\python27\python.exe x.py build --host x86_64-pc-windows-msvc --build x86_64-pc-windows-msvc --target thumbv7a-pc-windows-msvc --verbose
```

**Changes**
1. update cargolock to use compiler_builtins 0.1.15
2. handle libunwind in libtest for thumv7a the same as what we have for aarch64
3. in llvm codegen add a field in CodegenContext to carry the arch, so later in create_msvc_imps function, the arch can be used to check against "x86", instead of 32 pointer width. Apparently Thumv7a is handled differently than x86.

**Background**
I'm from Microsoft working on enabling Azure IoTEdge on ARM32 Windows IoTCore, Azure IoTEdge has a component called IoTEdged written in rust as a NT service running on Windows, so we need to enable rust on thumbv7a in order to have full IoTEdge. My colleague had made some heavy lifting and we've been using our private toolchain to build IoTEdged in our devops pipeline, because at that time we cannot build thumbv7a target end to end successfully. This change is a followup to enable the end to end build for thumbv7a-pc-windows-msvc target.

**Next step**
I'll submit more changes to have this target built nightly in rust/master, to achieve the same availability for aarch64-pc-windows-msvc, indexed here https://rust-lang.github.io/rustup-components-history/aarch64-pc-windows-msvc.html and can be manually installed. **Please do share what takes to make this happen, is there a formal process I need to follow\?**
bors added a commit that referenced this pull request May 20, 2019
Rollup of 6 pull requests

Successful merges:

 - #60590 (Test interaction of unions with non-zero/niche-filling optimization)
 - #60745 (Perform constant propagation into terminators)
 - #60895 (Enable thumbv7a-pc-windows-msvc target build end to end in rust/master)
 - #60908 (Fix lints handling in rustdoc)
 - #60960 (Stop using gensyms in HIR lowering)
 - #60962 (Fix data types indication)

Failed merges:

r? @ghost
@bors bors merged commit 759921e into rust-lang:master May 20, 2019
@chandde
Copy link
Contributor Author

chandde commented May 20, 2019

thanks for your review! now I'd like to have master branch build the target thumbv7a-pc-windows-msvc nightly, so user can install the target using rustup, without locally building the tool chain, is there instruction how to do it? ideally the same availability for aarch64-pc-windows-msvc here https://rust-lang.github.io/rustup-components-history/aarch64-pc-windows-msvc.html

@alexcrichton @mzji @mati865 @jonas-schievink

@alexcrichton
Copy link
Member

@chandde I think you'll basically want to grep around for aarch64-pc-windows-msvc in this repository and mirror its configuration, wherever aarch64 is mentioned thumbv7a probably also needs to be mentioned

@chandde
Copy link
Contributor Author

chandde commented May 20, 2019

I'll add thumbv7a to whereever aarch64 has been mentioned, i.e. follow this search https://github.com/search?q=aarch64-pc-windows-msvc+user%3Arust-lang+repo%3Arust&type=Code

Which exact file (from above search) that enables aarch64 for nightly build? once I make these changes locally, how do I verify? I'd expect I can verify things locally instead of keep pulling from next good nightly build..

Also where the builds happens? that I can take a look at build status, pipelines, steps, to get a better idea how a target is built

@alexcrichton

@alexcrichton
Copy link
Member

Currently the aarch64 target is built on AppVeyor, configured here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants