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 support for code model selection #4318

Merged
merged 1 commit into from Jan 29, 2020
Merged

Add support for code model selection #4318

merged 1 commit into from Jan 29, 2020

Conversation

SyrupThinker
Copy link
Contributor

@SyrupThinker SyrupThinker commented Jan 29, 2020

This pull request allows the user to select the code model that is targeted through the -code-model compiler flag or the LibExeObjStep.code_model field.
It is exposed through builtin.code_model.

Explanation:

In short the code model puts constraints on the location of symbols and the size of code and data.
The selection of a code model is a trade off on speed and restrictions that needs to be selected on a per application basis to meet its requirements.

A slightly more detailed explanation can be found in (for example) the System V Application Binary Interface (x86_64) 3.5.1.

Example:

An application compiled with the default amd64 code model (small) cannot be linked into negative address space because of the restrictions that it imposes.

Example for the linker error that would occur:

lld: error: start.zig:96:(.text+0x1401): relocation R_X86_64_32 out of range: 18446744073707835024 is not in [0, 4294967295]

The same code targeting the large or kernel code models would be allowed to link.

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch. Just a few things to clean up, and then I'll be happy to merge this.

Comment on lines 96 to 107
pub const CodeModel = enum {
Default,
Tiny,
Small,
Kernel,
Medium,
Large,
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New enums should follow #2101. This will be nice because you can use @tagName above instead of the switch.

@@ -1149,6 +1149,7 @@ pub const LibExeObjStep = struct {
name_prefix: []const u8,
filter: ?[]const u8,
single_threaded: bool,
code_model: ?builtin.CodeModel,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest:

Suggested change
code_model: ?builtin.CodeModel,
code_model: builtin.CodeModel = CodeModel.default,

and then the code below should omit the -code-model parameter when the value is default.

Comment on lines 2247 to 2248
LLVMCodeModel code_model;
CodeModel want_code_model;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we only need 1 field here:

CodeModel code_model;

codegen_create can default this to CodeModelDefault. main.cpp can override it.
The code that generates builtin.zig can switch on the CodeModel (not the LLVM one).
detect_code_model can be renamed to to_llvm_code_model, and only called once, for createTargetMachine.

The want_* mechanisms don't apply to this parameter, since it has a default and there is no logic in the "detect" phase.

Comment on lines +94 to +100
/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const CodeModel = enum {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add your handy explanation to the doc comments?

/// The code model puts constraints on the location of symbols and the size of code and data.
/// The selection of a code model is a trade off on speed and restrictions that needs to be selected on a per application basis to meet its requirements.
/// A slightly more detailed explanation can be found in (for example) the [System V Application Binary Interface (x86_64)](https://github.com/hjl-tools/x86-psABI/wiki/x86-64-psABI-1.0.pdf) 3.5.1.

@andrewrk andrewrk merged commit d448c3d into ziglang:master Jan 29, 2020
@andrewrk
Copy link
Member

Thanks!

@SyrupThinker SyrupThinker deleted the codemodel branch January 30, 2020 12:12
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.

None yet

2 participants