-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
add rustc option -Zpacked-stack #152432
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
base: main
Are you sure you want to change the base?
add rustc option -Zpacked-stack #152432
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -1348,6 +1348,12 @@ fn validate_commandline_args_with_session_available(sess: &Session) { | |||
| sess.dcx().emit_warn(errors::SoftFloatIgnored); | ||||
| } | ||||
| } | ||||
|
|
||||
| if sess.opts.unstable_opts.packed_stack { | ||||
| if sess.target.arch != Arch::S390x { | ||||
| sess.dcx().emit_err(errors::UnsupportedPackedStack); | ||||
| } | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like the most reasonable place to check the backchain condition?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes i will move the other check here
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please avoid duplicating the check across the codegen backends. The check seems to be somewhat similar in nature to
check_abi_required_features to something slightly more general and do the check there?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes this seems, also to me, the only option left.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is another way. I could try to talk to the llvm guys if its possible to make packed-stack a proper target-feature so we dont have to go over a cli argument.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if that would help. Target feature conflicts currently also need ad-hoc checks, we don't have a framework for dealing with them at the moment. |
||||
| } | ||||
| } | ||||
|
|
||||
| /// Holds data on the current incremental compilation session, if there is one. | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3258,6 +3258,16 @@ impl Target { | |
| )); | ||
| } | ||
| } | ||
| // If both `packed-stack` and `backchain` are set we need to use soft-float ABI. | ||
| if self.arch == Arch::S390x | ||
| && features_enabled.contains("packed-stack") | ||
| && features_enabled.contains("backchain") | ||
| && !matches!(self.abi, Abi::SoftFloat) | ||
| { | ||
| return Err(format!( | ||
| "Enabling both `packed-stack` and `backchain` attributes is incompatible with the default s390x target. Switch to s390x-unknown-none-softfloat if you need both attributes." | ||
| )); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This only checks the default-enabled features but ignores |
||
| } | ||
|
|
||
| Ok(()) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -487,6 +487,11 @@ one of the following values: | |
| If not specified, overflow checks are enabled if | ||
| [debug-assertions](#debug-assertions) are enabled, disabled otherwise. | ||
|
|
||
| ## packed-stack | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the option isn't stable yet, this is the wrong place to document it I think? |
||
|
|
||
| This flag enables packed StackFrames on s390x. Enabling both `packed-stack` and `backchain` attributes is incompatible | ||
| with the default s390x target. Switch to s390x-unknown-none-softfloat if you need both attributes. | ||
|
|
||
| ## panic | ||
|
|
||
| This option lets you control what happens when the code panics. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe panic here given that the frontend should already have emitted an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm
packed_stack_attr(...)is called every timellfn_attrs_from_instanceis called indifferent of target arch.And I think it would not be a good thing to add an arch check to
llfn_attrs_from_instanceas there are no other checks in that function. It blindly calls all possibilities and assumes that checks are already handled elsewhere.