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

Add i686-unknown-uefi target #64334

Merged
merged 1 commit into from Sep 12, 2019

Conversation

@jyao1
Copy link
Contributor

commented Sep 10, 2019

This adds a new rustc target-configuration called 'i686-unknown_uefi'.
This is similar to existing x86_64-unknown_uefi target.

The i686-unknown-uefi target can be used to build Intel Architecture
32bit UEFI application. The ABI defined in UEFI environment (aka IA32)
is similar to cdecl.

We choose i686-unknown-uefi-gnu instead of i686-unknown-uefi to avoid
the intrinsics generated by LLVM. The detail of root-cause and solution
analysis is added as comment in the code.
For x86_64-unknown-uefi, we cannot use -gnu, because the ABI between
MSVC and GNU is totally different, and UEFI chooses ABI similar to MSVC.
For i686-unknown-uefi, the UEFI chooses cdecl ABI, which is same as
MSVC and GNU. According to LLVM code, the only differences between MSVC
and GNU are fmodf(f32), longjmp() and TLS, which have no impact to UEFI.
As such, using i686-unknown-uefi-gnu is the simplest way to pass the build.

Adding the undefined symbols, such as _aulldiv() to rust compiler-builtins
is out of scope. But it may be considered later.

The scope of this patch is limited to support target-configuration.

No standard library support is added in this patch. Such work can be
done in future enhancement.

Cc: Josh Triplett josh.triplett@intel.com
Reviewed-by: Josh Triplett josh.triplett@intel.com

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Sep 10, 2019

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

This comment has been minimized.

Copy link
Collaborator

commented Sep 10, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed (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.
2019-09-10T05:46:45.0546588Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-09-10T05:46:45.0744274Z ##[command]git config gc.auto 0
2019-09-10T05:46:45.0821908Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-09-10T05:46:45.0878206Z ##[command]git config --get-all http.proxy
2019-09-10T05:46:45.1021325Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/64334/merge:refs/remotes/pull/64334/merge
---
2019-09-10T05:53:41.5776415Z    Compiling serde_json v1.0.40
2019-09-10T05:53:43.4567224Z    Compiling tidy v0.1.0 (/checkout/src/tools/tidy)
2019-09-10T05:53:54.3404941Z     Finished release [optimized] target(s) in 1m 29s
2019-09-10T05:53:54.3494055Z tidy check
2019-09-10T05:53:54.5032909Z tidy error: /checkout/src/librustc_target/spec/i686_unknown_uefi.rs:3: line longer than 100 chars
2019-09-10T05:53:54.5033937Z tidy error: /checkout/src/librustc_target/spec/i686_unknown_uefi.rs:81: line longer than 100 chars
2019-09-10T05:53:54.5034289Z tidy error: /checkout/src/librustc_target/spec/i686_unknown_uefi.rs:82: line longer than 100 chars
2019-09-10T05:53:56.3215278Z some tidy checks failed
2019-09-10T05:53:56.3221303Z 
2019-09-10T05:53:56.3221303Z 
2019-09-10T05:53:56.3222619Z 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"
2019-09-10T05:53:56.3223150Z 
2019-09-10T05:53:56.3223266Z 
2019-09-10T05:53:56.3229491Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
2019-09-10T05:53:56.3229933Z Build completed unsuccessfully in 0:01:32
2019-09-10T05:53:56.3229933Z Build completed unsuccessfully in 0:01:32
2019-09-10T05:53:56.3283365Z == clock drift check ==
2019-09-10T05:53:56.3298757Z   local time: Tue Sep 10 05:53:56 UTC 2019
2019-09-10T05:53:56.4022934Z   network time: Tue, 10 Sep 2019 05:53:56 GMT
2019-09-10T05:53:56.4025567Z == end clock drift check ==
2019-09-10T05:53:57.8546981Z ##[error]Bash exited with code '1'.
2019-09-10T05:53:57.8580959Z ##[section]Starting: Checkout
2019-09-10T05:53:57.8582963Z ==============================================================================
2019-09-10T05:53:57.8583033Z Task         : Get sources
2019-09-10T05:53:57.8583074Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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)

Add i686-unknown-uefi target
This adds a new rustc target-configuration called 'i686-unknown_uefi'.
This is similar to existing x86_64-unknown_uefi target.

The i686-unknown-uefi target can be used to build Intel Architecture
32bit UEFI application. The ABI defined in UEFI environment (aka IA32)
is similar to cdecl.

We choose i686-unknown-uefi-gnu instead of i686-unknown-uefi to avoid
the intrinsics generated by LLVM. The detail of root-cause and solution
analysis is added as comment in the code.
For x86_64-unknown-uefi, we cannot use -gnu, because the ABI between
MSVC and GNU is totally different, and UEFI chooses ABI similar to MSVC.
For i686-unknown-uefi, the UEFI chooses cdecl ABI, which is same as
MSVC and GNU. According to LLVM code, the only differences between MSVC
and GNU are fmodf(f32), longjmp() and TLS, which have no impact to UEFI.
As such, using i686-unknown-uefi-gnu is the simplest way to pass the build.

Adding the undefined symbols, such as _aulldiv() to rust compiler-builtins
is out of scope. But it may be considered later.

The scope of this patch is limited to support target-configuration.

No standard library support is added in this patch. Such work can be
done in future enhancement.

Cc: Josh Triplett <josh.triplett@intel.com>
Reviewed-by: Josh Triplett <josh.triplett@intel.com>

@jyao1 jyao1 force-pushed the jyao1:i686-master branch from ce7931c to 21e062d Sep 10, 2019

@joshtriplett

This comment has been minimized.

Copy link
Member

commented Sep 11, 2019

Sorry, didn't see this when it was first posted.

This looks good to me, and will allow further development on this target. Once merged, we can experiment further outside of rustc (using cargo xbuild and similar), and work towards implementing core and later (most of) std for cfg(target_os="uefi").

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2019

📌 Commit 21e062d has been approved by joshtriplett

@bors

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2019

⌛️ Testing commit 21e062d with merge c9edc02...

bors added a commit that referenced this pull request Sep 11, 2019
Auto merge of #64334 - jyao1:i686-master, r=joshtriplett
Add i686-unknown-uefi target

This adds a new rustc target-configuration called 'i686-unknown_uefi'.
This is similar to existing x86_64-unknown_uefi target.

The i686-unknown-uefi target can be used to build Intel Architecture
32bit UEFI application. The ABI defined in UEFI environment (aka IA32)
is similar to cdecl.

We choose i686-unknown-uefi-gnu instead of i686-unknown-uefi to avoid
the intrinsics generated by LLVM. The detail of root-cause and solution
analysis is added as comment in the code.
For x86_64-unknown-uefi, we cannot use -gnu, because the ABI between
MSVC and GNU is totally different, and UEFI chooses ABI similar to MSVC.
For i686-unknown-uefi, the UEFI chooses cdecl ABI, which is same as
MSVC and GNU. According to LLVM code, the only differences between MSVC
and GNU are fmodf(f32), longjmp() and TLS, which have no impact to UEFI.
As such, using i686-unknown-uefi-gnu is the simplest way to pass the build.

Adding the undefined symbols, such as _aulldiv() to rust compiler-builtins
is out of scope. But it may be considered later.

The scope of this patch is limited to support target-configuration.

No standard library support is added in this patch. Such work can be
done in future enhancement.

Cc: Josh Triplett <josh.triplett@intel.com>
Reviewed-by: Josh Triplett <josh.triplett@intel.com>
@bors

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

☀️ Test successful - checks-azure
Approved by: joshtriplett
Pushing c9edc02 to master...

@bors bors added the merged-by-bors label Sep 12, 2019

@bors bors merged commit 21e062d into rust-lang:master Sep 12, 2019

5 checks passed

homu Test successful
Details
pr Build #20190910.11 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (LinuxTools) LinuxTools succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.