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.

Sign up for GitHub

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

Jump to bottom

Revert SGX inline asm syntax #85054

Merged
merged 1 commit into from
Jun 22, 2021
Merged

Revert SGX inline asm syntax #85054

merged 1 commit into from
Jun 22, 2021

Conversation

jethrogb
Copy link
Contributor

@jethrogb jethrogb commented May 7, 2021

This was erroneously changed in #83387

Revert SGX inline asm syntax
5bbf8cf 
This was erroneously changed in rust-lang#83387
@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 7, 2021
@bjorn3
Copy link
Member

bjorn3 commented May 10, 2021

AT&T syntax was used to support LLVM 8 and 9. Now that the minimum supported LLVM is 10, intel syntax is being used everywhere.

@jethrogb
Copy link
Contributor Author

AT&T syntax was used to support LLVM 8 and 9.

No, AT&T syntax was used as this is the standard for inline assembly. That LLVM 10 supports Intel syntax is not a reason to rewrite everything.

@bjorn3
Copy link
Member

bjorn3 commented May 10, 2021

The inline asm rfc chose intel syntax as default. That is why there is an att_syntax flag instead of an intel_syntax flag.

https://rust-lang.github.io/rfcs/2873-inline-asm.html

Additionally, this RFC proposes using the Intel asm syntax by default on x86 instead of the AT&T syntax. We believe this syntax will be more familiar to most users, but may be surprising for users used to GCC-style asm.

[...]

Use AT&T syntax by default on x86

x86 is particular in that there are two widely used dialects for its assembly code: Intel syntax, which is the official syntax for x86 assembly, and AT&T syntax which is used by GCC (via GAS). There is no functional difference between those two dialects, they both support the same functionality but with a different syntax. This RFC chooses to use Intel syntax by default since it is more widely used and users generally find it easier to read and write.

@jethrogb
Copy link
Contributor Author

The RFC merely describes the default behavior of the asm! macro.

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 31, 2021
@JohnTitor
Copy link
Member

r? @Amanieu as you're familiar with it and may have some thoughts.

@Amanieu
Copy link
Member

Amanieu commented Jun 21, 2021

It doesn't really matter either way, the end result is the same. I'll leave the asm style up to the discretion of the SGX target maintainers.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 21, 2021

📌 Commit 5bbf8cf has been approved by Amanieu

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Jun 21, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 21, 2021
@JohnTitor
Rollup merge of rust-lang#85054 - jethrogb:jb/sgx-inline-asm, r=Amanieu
dfeb064 
Revert SGX inline asm syntax

This was erroneously changed in rust-lang#83387
This was referenced Jun 21, 2021
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 22, 2021
@bors
Auto merge of rust-lang#86527 - JohnTitor:rollup-cbu78g4, r=JohnTitor
2c04f0b 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#85054 (Revert SGX inline asm syntax)
 - rust-lang#85182 (Move `available_concurrency` implementation to `sys`)
 - rust-lang#86037 (Add `io::Cursor::{remaining, remaining_slice, is_empty}`)
 - rust-lang#86114 (Reopen rust-lang#79692 (Format symbols under shared frames))
 - rust-lang#86297 (Allow to pass arguments to rustdoc-gui tool)
 - rust-lang#86334 (Resolve type aliases to the type they point to in intra-doc links)
 - rust-lang#86367 (Fix comment about rustc_inherit_overflow_checks in abs().)
 - rust-lang#86381 (Add regression test for issue rust-lang#39161)
 - rust-lang#86387 (Remove `#[allow(unused_lifetimes)]` which is now unnecessary)
 - rust-lang#86398 (Add regression test for issue rust-lang#54685)
 - rust-lang#86493 (Say "this enum variant takes"/"this struct takes" instead of "this function takes")

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d6e344d into rust-lang:master Jun 22, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jun 22, 2021
@workingjubilee workingjubilee added the O-SGX Target: SGX label Jul 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Amanieu Amanieu

Labels
O-SGX Target: SGX S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.55.0
Development

Successfully merging this pull request may close these issues.
10 participants
@jethrogb @rust-highfive @bjorn3 @JohnTitor @Amanieu @bors @joshtriplett @JohnCSimon @workingjubilee @rustbot