Use small code model for UEFI targets #87124

Andy-Python-Programmer · 2021-07-14T09:21:13Z

Since the code model only applies to the code and not the data and the code model
only applies to functions you call through using call, jmp and data with lea, etc…

If you are calling functions using the function pointers from the UEFI structures the code
model does not apply in that case. It’s just related to the address space size of your own binary.
Since UEFI (uefi is all relocatable) uses relocatable PEs (relocatable code does not care about the
code model) so, we use the small code model here.
Since applications don't usually take gigabytes of memory, setting the
target to use the small code model should result in better codegen (comparable
with majority of other targets).

Large code models are also known for generating horrible code, for
example 16 bytes of code to load a single 8-byte value.

Signed-off-by: Andy-Python-Programmer andypythonappdeveloper@gmail.com

rust-highfive · 2021-07-14T09:21:16Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @oli-obk (or someone else) soon.

Please see the contribution instructions for more information.

rust-highfive · 2021-07-14T09:21:17Z

⚠️ Warning ⚠️

These commits modify compiler targets. (See the Target Tier Policy.)

oli-obk · 2021-07-14T10:12:18Z

This sounds reasonable, but I know nothing about code models or UEFI

cc @rust-lang/wg-llvm any takers?

nagisa · 2021-07-14T11:09:31Z

This seems reasonable enough to me, but I also wonder if it shouldn't be up to the user to apply this optimization (since it can indeed make builds fail for people building large UEFI applications). I've a hard time believing that the improvement here is significant enough for most people writing UEFI applications to care.

Andy-Python-Programmer · 2021-07-14T11:31:24Z

This pull can be classified as a follow up pull request to #85357, where we actually have to use the small code model (as as you can see in the comments) rust performs some relocations (when compiling core) that are not supported on COFF targets. This is not an issue on x86_64 but to keep the target code models equal I guess we should use the small code model here too. And

Since UEFI (uefi is all relocatable) uses relocatable PEs (relocatable code does not care about the
code model) so, we use the small code model here.

dvdhrm · 2021-07-14T12:11:18Z

As I mentioned in #85357, there is only very limited information on the code model knob and so far no-one could explain to us what this really controls, why the compiler cannot guess it, or what a safe default would be.

Apart from this, I wonder whether it is the right fix. Should'nt we rather try to fix the wrong relocations of rustc?

compiler/rustc_target/src/spec/x86_64_unknown_uefi.rs

petrochenkov · 2021-07-14T12:22:50Z

@dvermd
At least one of the questions has an easy answer:

why the compiler cannot guess it

The fact that the produced executable/dll is going to be huge can be discovered only during linking when multiple object files are combined together, but all the compiler's work is already done.

bjorn3 · 2021-07-14T17:39:30Z

For reference small is the default code model on x86 in LLVM.

https://github.com/llvm/llvm-project/blob/7de2173c2a4c45711831cfee3ccf53690c76ff07/llvm/lib/Target/X86/X86TargetMachine.cpp#L204

Andy-Python-Programmer · 2021-07-15T07:00:41Z

Now rather then explicitly setting the code model to small, we pick let LLVM to pick the code model (which is the default code model for the architecture).

Should'nt we rather try to fix the wrong relocations of rustc?

This should be an issue in rustc, I suspect as the code model does not matter if your code is relocatable. This pull request just makes sure that we use the default LLVM code model to improve codegen since as code model does not matter if your code is relocatable:

Large code models are also known for generating horrible code, for
example 16 bytes of code to load a single 8-byte value.

petrochenkov · 2021-07-15T12:14:14Z

I'm ready to r+ this because it looks generally reasonable, but I'd want to make sure that this was actually tested on x86 UEFI.

Andy-Python-Programmer · 2021-07-16T01:49:00Z

I'm ready to r+ this because it looks generally reasonable, but I'd want to make sure that this was actually tested on x86 UEFI.

Yep, I did run the uefi-rs tests and they all succeed:

andy@DESKTOP-IUH6DM6:~/uefi-rs/uefi-test-runner$ ./build.py --target x86_64 --headless --ci run
       Fresh core v0.0.0 (/home/andy/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/src/rust/library/core)
       Fresh unicode-xid v0.2.2
       Fresh rustc-std-workspace-core v1.99.0 (/home/andy/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/src/rust/library/rustc-std-workspace-core)
       Fresh compiler_builtins v0.1.46
       Fresh alloc v0.0.0 (/home/andy/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/src/rust/library/alloc)
       Fresh proc-macro2 v1.0.27
       Fresh quote v1.0.9
       Fresh cfg-if v1.0.0
       Fresh bit_field v0.10.1
       Fresh bitflags v1.2.1
       Fresh qemu-exit v2.0.1
       Fresh rlibc v1.0.0
       Fresh syn v1.0.73
       Fresh ucs2 v0.3.2
       Fresh log v0.4.14
       Fresh uefi-macros v0.3.3 (/home/andy/uefi-rs/uefi-macros)
       Fresh uefi v0.11.0 (/home/andy/uefi-rs)
       Fresh uefi-services v0.8.0 (/home/andy/uefi-rs/uefi-services)
       Fresh uefi-test-runner v0.2.0 (/home/andy/uefi-rs/uefi-test-runner)
    Finished dev [unoptimized + debuginfo] target(s) in 0.12s
BdsDxe: loading Boot0001 "UEFI QEMU HARDDISK QM00001 " from PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)
BdsDxe: starting Boot0001 "UEFI QEMU HARDDISK QM00001 " from PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)
INFO: UEFI 2.7
INFO: Testing boot services
INFO: Testing memory functions
INFO: Allocating some pages of memory
INFO: Allocating a vector through the `alloc` crate
INFO: Allocating a structure with alignment to 0x100
INFO: Testing the `memmove` / `set_mem` functions
INFO: Testing memory map functions
INFO: Testing timer...
INFO: Testing watchdog...
INFO: Testing various protocols
INFO: Testing console protocols
INFO: Running text output protocol test
INFO: UEFI standard output current mode: Some(OutputMode { index: 0, dims: (80, 25) })
INFO: # uefi-rs test runner
INFO: Cursor visibility control unavailable
INFO: - Text mode #0: 25 rows by 80 columns
INFO: - Text mode #1: 31 rows by 100 columns
INFO: Running serial protocol test
INFO: Running graphics output protocol test
SCREENSHOT: gop_test
INFO: Running pointer protocol test
INFO: Pointer state has not changed since the last query
INFO: Running UEFI debug connection protocol test
INFO: - Architecture: EBC
INFO: Testing Media Access protocols
INFO: Root directory entry: NamedFileProtocolInfo { header: FileInfoHeader { size: 88, file_size: 8192, physical_size: 8192, create_time: 2021-7-16 11:34:32.0 2047 (empty), last_access_time: 2021-7-16 0:0:0.0 2047 (empty), modification_time: 2021-7-16 11:34:32.0 2047 (empty), attribute: DIRECTORY }, name: ['E', 'F', 'I', '\u{0}'] }
INFO: MBR partition: MbrPartitionRecord { boot_indicator: 128, starting_chs: [1, 1, 0], os_type: MbrOsType(6), ending_chs: [15, 255, 255], starting_lba: 63, size_in_lba: 1032129 }
INFO: Testing Platform Initialization protocols
INFO: Running shim lock protocol test
INFO: Shim lock protocol is not supported
INFO: Testing runtime services
INFO: Testing set_variable
INFO: Testing get_variable_size
INFO: Testing get_variable
INFO: Testing complete, shutting down...
andy@DESKTOP-IUH6DM6:~/uefi-rs/uefi-test-runner$

The only change I made to the build script was adding +aarch-dev and --verbose arguments to cargo. Where +aarch-dev is where I have linked my local toolchain:

python3 ./x.py build -i library/std && rustup toolchain link aarch-dev build/x86_64-unknown-linux-gnu/stage1

Since I am on windows, I ran the tests on WSL without KVM which the tests script (the test script by default uses KVM acceleration). I added the --ci option to the build script to avoid running the tests with KVM acceleration as its not supported on WSL itself.

I have also tested my bootloader and it works perfectly fine. I also flashed my bootloader into USB and booted it and works fine too.

petrochenkov · 2021-07-16T09:43:46Z

Ok, r=me with commits squashed.

* Since the code model only applies to the code and not the data and the code model only applies to functions you call through using `call`, `jmp` and data with `lea`, etc… If you are calling functions using the function pointers from the UEFI structures the code model does not apply in that case. It’s just related to the address space size of your own binary. Since UEFI (uefi is all relocatable) uses relocatable PEs (relocatable code does not care about the code model) so, we use the small code model here. * Since applications don't usually take gigabytes of memory, setting the target to use the small code model should result in better codegen (comparable with majority of other targets). Large code models are also known for generating horrible code, for example 16 bytes of code to load a single 8-byte value. * Use the LLVM default code model for the architecture for the x86_64-unknown-uefi targets. For reference small is the default code model on x86 in LLVM: <https://github.com/llvm/llvm-project/blob/7de2173c2a4c45711831cfee3ccf53690c76ff07/llvm/lib/Target/X86/X86TargetMachine.cpp#L204> * Remove the comments too as they are not UEFI-specific and applies to pretty much any target. I added them before as I was explicitily setting the code model to small. Signed-off-by: Andy-Python-Programmer <andypythonappdeveloper@gmail.com>

Andy-Python-Programmer · 2021-07-17T04:14:06Z

Ok, r=me with commits squashed.

Done, squashed commits...

petrochenkov · 2021-07-17T08:46:20Z

@bors r+

bors · 2021-07-17T08:46:21Z

📌 Commit db1e492 has been approved by petrochenkov

bors · 2021-07-17T10:15:38Z

⌛ Testing commit db1e492 with merge 64d171b...

bors · 2021-07-17T12:56:16Z

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 64d171b to master...

There is no need to use the large code model. See: rust-lang/rust#87124

rust-highfive assigned oli-obk Jul 14, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 14, 2021

petrochenkov reviewed Jul 14, 2021

View reviewed changes

compiler/rustc_target/src/spec/x86_64_unknown_uefi.rs Outdated Show resolved Hide resolved

oli-obk removed their assignment Jul 14, 2021

This comment has been minimized.

Sign in to view

Andy-Python-Programmer requested a review from petrochenkov July 15, 2021 07:09

petrochenkov self-assigned this Jul 15, 2021

petrochenkov approved these changes Jul 15, 2021

View reviewed changes

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 15, 2021

Andy-Python-Programmer force-pushed the code_model_uefi_patch branch from ebe9062 to db1e492 Compare July 17, 2021 04:13

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 17, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 17, 2021

bors merged commit 64d171b into rust-lang:master Jul 17, 2021

rustbot added this to the 1.55.0 milestone Jul 17, 2021

Andy-Python-Programmer deleted the code_model_uefi_patch branch August 9, 2021 23:53

toku-sa-n added a commit to toku-sa-n/antei that referenced this pull request Aug 10, 2021

chore(bootx64): use the small code model

087321d

There is no need to use the large code model. See: rust-lang/rust#87124

toku-sa-n mentioned this pull request Aug 10, 2021

chore(bootx64): use the small code model toku-sa-n/antei#177

Merged

toku-sa-n added a commit to toku-sa-n/antei that referenced this pull request Aug 10, 2021

chore(bootx64): use the small code model (#177)

52ec9ae

There is no need to use the large code model. See: rust-lang/rust#87124

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use small code model for UEFI targets #87124

Use small code model for UEFI targets #87124

Andy-Python-Programmer commented Jul 14, 2021 •

edited

Loading

rust-highfive commented Jul 14, 2021

rust-highfive commented Jul 14, 2021

oli-obk commented Jul 14, 2021

nagisa commented Jul 14, 2021

Andy-Python-Programmer commented Jul 14, 2021 •

edited

Loading

dvdhrm commented Jul 14, 2021

petrochenkov commented Jul 14, 2021

bjorn3 commented Jul 14, 2021

This comment has been minimized.

Andy-Python-Programmer commented Jul 15, 2021 •

edited

Loading

petrochenkov commented Jul 15, 2021

Andy-Python-Programmer commented Jul 16, 2021 •

edited

Loading

petrochenkov commented Jul 16, 2021

Andy-Python-Programmer commented Jul 17, 2021

petrochenkov commented Jul 17, 2021

bors commented Jul 17, 2021

bors commented Jul 17, 2021

bors commented Jul 17, 2021

Use small code model for UEFI targets #87124

Use small code model for UEFI targets #87124

Conversation

Andy-Python-Programmer commented Jul 14, 2021 • edited Loading

rust-highfive commented Jul 14, 2021

rust-highfive commented Jul 14, 2021

oli-obk commented Jul 14, 2021

nagisa commented Jul 14, 2021

Andy-Python-Programmer commented Jul 14, 2021 • edited Loading

dvdhrm commented Jul 14, 2021

petrochenkov commented Jul 14, 2021

bjorn3 commented Jul 14, 2021

This comment has been minimized.

Andy-Python-Programmer commented Jul 15, 2021 • edited Loading

petrochenkov commented Jul 15, 2021

Andy-Python-Programmer commented Jul 16, 2021 • edited Loading

petrochenkov commented Jul 16, 2021

Andy-Python-Programmer commented Jul 17, 2021

petrochenkov commented Jul 17, 2021

bors commented Jul 17, 2021

bors commented Jul 17, 2021

bors commented Jul 17, 2021

Andy-Python-Programmer commented Jul 14, 2021 •

edited

Loading

Andy-Python-Programmer commented Jul 14, 2021 •

edited

Loading

Andy-Python-Programmer commented Jul 15, 2021 •

edited

Loading

Andy-Python-Programmer commented Jul 16, 2021 •

edited

Loading