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

Disable stack probes #363

Merged
merged 1 commit into from
Jun 7, 2021
Merged

Disable stack probes #363

merged 1 commit into from
Jun 7, 2021

Conversation

nbdd0121
Copy link
Member

@nbdd0121 nbdd0121 commented Jun 6, 2021

We do not support function-call stack probes, and LLVM inline asm
stack probe is currently broken 1 2. Switching target's stack-probes
to "none" allows us to avoid broken codegen and the intrinsic call.
We could switch the stack-probes property to inline if we want once
the LLVM codegen issue is resolved.

@ksquirrel

This comment has been minimized.

We do not support function-call stack probes, and LLVM inline asm stack
probe is currently broken [1][2]. Switching target's `stack-probes`
to "none" allows us to avoid broken codegen and the intrinsic call.
We could switch the `stack-probes` property to `inline` if we want once
the LLVM codegen issue is resolved.

[1]: https://bugs.llvm.org/show_bug.cgi?id=50165
[2]: rust-lang/rust#84667

Signed-off-by: Gary Guo <[email protected]>
@ksquirrel
Copy link
Member

Review of 53bc4d452e2f:

  • ✔️ Commit 53bc4d4: Looks fine!

@ojeda
Copy link
Member

ojeda commented Jun 7, 2021

Is there an issue number tracking the inline support?

Comment on lines -45 to -47
define_panicking_intrinsics!("non-inline stack probes should not be used", {
__rust_probestack,
});
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to remove this? i.e. can we keep it as a safeguard?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's best to remove this so we know there're no calls generated by accident. Note that we will never need __rust_probestack in the future -- once LLVM bug is fixed we'll switch to inline stack probes.

Copy link
Member

@bjorn3 bjorn3 Jun 7, 2021

Choose a reason for hiding this comment

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

Removing this ensures that non-inline stack probes are never generated.

Edit: race-condition :)

@ojeda ojeda merged commit 6470c0c into Rust-for-Linux:rust Jun 7, 2021
@nbdd0121
Copy link
Member Author

nbdd0121 commented Jun 7, 2021

There does not seem to be a tracking issue for inline stack probes on Rust side.

@ojeda
Copy link
Member

ojeda commented Jun 7, 2021

Updated #355 with the LLVM one then.

@nbdd0121 nbdd0121 deleted the intrinsics branch June 10, 2021 20:45
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.

4 participants