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

rust: generate target specification files on the fly #694

Merged
merged 1 commit into from
Mar 4, 2022

Conversation

ojeda
Copy link
Member

@ojeda ojeda commented Mar 3, 2022

Now that we have host programs support, add a "Rust script"
to generate the rustc target spec file.

The script has been written so that its output matches exactly
the current arch/*/rust/*.json static target files we have
at the moment, to have a baseline and reduce the changes done
by this commit. Thus the static files are now gone.

The build system is set up in a way that we do not need to rebuild
any Rust code as long as the target.json contents do not change,
even if the kernel configuration changes for something else.
Furthermore, the script does not need to be rebuilt even if
the kernel configuration changes.

With this, KBUILD_RUST_TARGET is also gone -- one should modify
the script instead of trying to override the target file.

After this, the script should be cleaned (e.g. remove unneeded
keys) and fixed to configure the architectures properly (e.g.
enable CPU features as needed, etc.).

There were several design choices:

  • The language: this could have been done in a shell script,
    C or Perl. But as soon as Rust is an option, it is more
    convenient than any of those. And it is an ideal showcase for
    the Rust host programs support.

    Python could have also been a natural option for the script,
    though it cannot be used as part as the kernel compilation
    because it is not required (as far as I can see in the docs).
    Even if it were, Rust offers some advantages anyway (though
    the json module would have been fairly useful here).

    Compiling the Rust host program takes a lot of time compared
    to other languages, at least currently (e.g. 300ms), but there
    is no need to recompile it on config changes, so it is fine.
    And, of course, running it is very fast.

  • Whether or not to use conditional compilation.

    Initially I took advantage of the rustc_cfg flags, to make
    the script look closer to the actual Rust kernel code.

    However, that means the script depends on the config, which
    means it is not an independent tool anymore (though there is
    at least one other host program that gets passed the flags,
    but it is not common), plus requires recompiling it every
    time the configuration changes just to generate the actual
    file. It also made things more tricky on the build system.

  • Splitting the logic into arch/* or not.

    In order to have arch maintainers work independently, we could
    move part of the logic into their trees.

    However, given how small the script is and that some logic
    is shared anyway, it looks a little bit odd. So I decided
    to avoid it. We can revisit this if needed.

  • Whether to use a proper JSON generator or not, whether to
    use a HashMap, whether to set up a type that knows about
    all the keys that can be generated (or maybe even import
    part of the rustc code to be in sync), etc.

    The script has been written to be as simple as possible
    while making it easy to change the actual business logic.
    We should reevaluate as it gets developed.

Signed-off-by: Miguel Ojeda [email protected]

Now that we have host programs support, add a "Rust script"
to generate the `rustc` target spec file.

The script has been written so that its output matches exactly
the current `arch/*/rust/*.json` static target files we have
at the moment, to have a baseline and reduce the changes done
by this commit. Thus the static files are now gone.

The build system is set up in a way that we do not need to rebuild
any Rust code as long as the `target.json` contents do not change,
even if the kernel configuration changes for something else.
Furthermore, the script does not need to be rebuilt even if
the kernel configuration changes.

With this, `KBUILD_RUST_TARGET` is also gone -- one should modify
the script instead of trying to override the target file.

After this, the script should be cleaned (e.g. remove unneeded
keys) and fixed to configure the architectures properly (e.g.
enable CPU features as needed, etc.).

There were several design choices:

  - The language: this could have been done in a shell script,
    C or Perl. But as soon as Rust is an option, it is more
    convenient than any of those. And it is an ideal showcase for
    the Rust host programs support.

    Python could have also been a natural option for the script,
    though it cannot be used as part as the kernel compilation
    because it is not required (as far as I can see in the docs).
    Even if it were, Rust offers some advantages anyway (though
    the `json` module would have been fairly useful here).

    Compiling the Rust host program takes a lot of time compared
    to other languages, at least currently (e.g. 300ms), but there
    is no need to recompile it on config changes, so it is fine.
    And, of course, running it is very fast.

  - Whether or not to use conditional compilation.

    Initially I took advantage of the `rustc_cfg` flags, to make
    the script look closer to the actual Rust kernel code.

    However, that means the script depends on the config, which
    means it is not an independent tool anymore (though there is
    at least one other host program that gets passed the flags,
    but it is not common), plus requires recompiling it every
    time the configuration changes just to generate the actual
    file. It also made things more tricky on the build system.

  - Splitting the logic into `arch/*` or not.

    In order to have arch maintainers work independently, we could
    move part of the logic into their trees.

    However, given how small the script is and that some logic
    is shared anyway, it looks a little bit odd. So I decided
    to avoid it. We can revisit this if needed.

  - Whether to use a proper JSON generator or not, whether to
    use a `HashMap`, whether to set up a type that knows about
    all the keys that can be generated (or maybe even import
    part of the `rustc` code to be in sync), etc.

    The script has been written to be as simple as possible
    while making it easy to change the actual business logic.
    We should reevaluate as it gets developed.

Signed-off-by: Miguel Ojeda <[email protected]>
@bjorn3
Copy link
Member

bjorn3 commented Mar 3, 2022

I will take a look tomorrow.

@ojeda
Copy link
Member Author

ojeda commented Mar 4, 2022

Thanks a lot! No rush.

match self {
Value::Boolean(boolean) => write!(formatter, "{}", boolean),
Value::Number(number) => write!(formatter, "{}", number),
Value::String(string) => write!(formatter, "\"{}\"", string),
Copy link
Member

Choose a reason for hiding this comment

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

You could try {:?}. This will add quotes around the string and escape at least things like quotes. It doesn't exactly match json, but it may be a bit less likely to trip up a json parser than what you have right now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, I had to change this due to the "\u0001__gnu_mcount_nc" value ({:?} escapes backslashes).

"e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128",
);
ts.push("disable-redzone", true);
ts.push("emit-debug-gdb-scripts", false);
Copy link
Member

Choose a reason for hiding this comment

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

This one should probably be moved out of the target match.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but please see the commit message, i.e. the intention of this PR is to keep the targets exactly the same as they are now, so that this commit is focused only about introducing the script, and then clean things up. Same for the other bits.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense.

ts.push("max-atomic-width", 128);
ts.push("needs-plt", true);
ts.push("pre-link-args", pre_link_args_64);
ts.push("stack-probes", stack_probes);
Copy link
Member

Choose a reason for hiding this comment

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

This one too I think.

ts.push("disable-redzone", true);
ts.push("emit-debug-gdb-scripts", false);
ts.push("features", "+strict-align,+neon,+fp-armv8");
ts.push("frame-pointer", "always");
Copy link
Member

Choose a reason for hiding this comment

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

And this one.

}
ts.push("code-model", "medium");
ts.push("disable-redzone", true);
ts.push("emit-debug-gdb-scripts", false);
Copy link
Member

Choose a reason for hiding this comment

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

And this one.

ts.push("env", "gnu");
ts.push("function-sections", false);
ts.push("linker-is-gnu", true);
ts.push("os", if cfg.has("ARM") { "linux" } else { "none" });
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this always be "none"?

"e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128",
);
ts.push("disable-redzone", true);
ts.push("emit-debug-gdb-scripts", false);
Copy link
Member

Choose a reason for hiding this comment

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

Makes sense.

@ojeda
Copy link
Member Author

ojeda commented Mar 4, 2022

@bjorn3 Thanks a lot for the review, as usual!

@ojeda ojeda merged commit afa6026 into Rust-for-Linux:rust Mar 4, 2022
@ojeda ojeda deleted the target branch March 4, 2022 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants