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

Add v9, v8plus, and leoncasa target feature to sparc and use v8plus in create_object_file #132552

Merged
merged 2 commits into from
Nov 9, 2024
Merged

Add v9, v8plus, and leoncasa target feature to sparc and use v8plus in create_object_file #132552

merged 2 commits into from
Nov 9, 2024

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Nov 3, 2024 •
edited by workingjubilee
Loading

This adds the following three unstable target features:

cc @thejpster @glaubitz

r? workingjubilee

Closes #132585

@rustbot label +O-SPARC +A-target-feature

Footnotes

  1. Atomic CAS instruction ↩

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. O-SPARC Target: SPARC processors labels Nov 3, 2024
@taiki-e taiki-e mentioned this pull request Nov 3, 2024
Merged
@taiki-e taiki-e changed the title Add v9 and hasleoncasa target feature to sparc Add v9 and leoncasa target feature to sparc Nov 3, 2024
@rustbot rustbot added the A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. label Nov 3, 2024
@glaubitz
Copy link
Contributor

glaubitz commented Nov 3, 2024

CC @psumbera @rorth @koachan @jrtc27 @andreas-gaisler

@koachan
Copy link

koachan commented Nov 3, 2024

I'm not very familiar with Rust, but from what I can tell it looks okay.

Also, a word on this:

My understanding is that (some) use of the v9 feature may eventually be replaced by the v8plus feature in LLVM 20 (llvm/llvm-project@aca971d), but there is no other good way in the current LLVM 19.

v8plus is a purely ABI flag, intended to signal that the G and O registers are 64-bit. It is separated in LLVM because we want to be GCC-compatible and so need to cater to a particular usecase in which V9 instructions are desired, but all registers are to be kept 32-bit (effectively banning instructions like CASXA which needs 64-bit registers to operate).
If Rust doesn't want to do this, it could always set (or delete) v9 and v8plus together...

@taiki-e
Copy link
Member Author

taiki-e commented Nov 3, 2024 •
edited
Loading

v8plus is a purely ABI flag, intended to signal that the G and O registers are 64-bit. It is separated in LLVM because we want to be GCC-compatible and so need to cater to a particular usecase in which V9 instructions are desired, but all registers are to be kept 32-bit (effectively banning instructions like CASXA which needs 64-bit registers to operate). If Rust doesn't want to do this, it could always set (or delete) v9 and v8plus together...

Thanks for the clarification. I think separating v8plus from v9 is the correct behavior (eventually) for Rust as well, as we do not want the ABI to be changed by -C target-cpu (-mtune).

However, we are using LLVM 19 which ties v9 and v8plus together and need a way to fix a bug in rustc that incorrectly always uses EM_SPARC32PLUS (#130172).

In #131222 we are trying to adopt the same approach as in LLVM 19, but actually adding the v8plus feature and mapping them to the v9 feature in LLVM 19 may be a better way here. (The implementation of it is in 0849356 and even if we merge the LLVM 19's approach, we will probably need it when updating to LLVM 20.)

@workingjubilee workingjubilee mentioned this pull request Nov 4, 2024
Closed
@bors
Copy link
Contributor

bors commented Nov 5, 2024

☔ The latest upstream changes (presumably #129884) made this pull request unmergeable. Please resolve the merge conflicts.

@taiki-e taiki-e mentioned this pull request Nov 7, 2024
Merged
@rustbot
Copy link
Collaborator

rustbot commented Nov 7, 2024

These commits modify compiler targets.
(See the Target Tier Policy.)

@taiki-e taiki-e force-pushed the sparc-target-feature branch 3 times, most recently from 2597e97 to 81374d7 Compare November 7, 2024 12:51
@taiki-e taiki-e changed the title Add v9 and leoncasa target feature to sparc Add v9, v8plus, and leoncasa target feature to sparc Nov 7, 2024
@taiki-e taiki-e changed the title Add v9, v8plus, and leoncasa target feature to sparc Add v9, v8plus, and leoncasa target feature to sparc and use v8plus in create_object_file Nov 7, 2024
@taiki-e
Copy link
Member Author

taiki-e commented Nov 7, 2024

Added commit to add v8plus target feature and use it (the same one that mentioned in #132552 (comment)). This fixes #132585 and should work the same way as LLVM in each LLVM version. (See #132585 (comment) for more info.)

(If it would be better to separate that commit into a separate PR, I would be happy to separate it.)

@workingjubilee
Copy link
Member

I think fixing that here should be fine?

Can we get a separate tracking issue for the SPARC target features though?

@taiki-e taiki-e mentioned this pull request Nov 8, 2024
Open
4 tasks
@taiki-e
Copy link
Member Author

taiki-e commented Nov 8, 2024

Can we get a separate tracking issue for the SPARC target features though?

Opened #132783 and updated the tracking issue number.

@workingjubilee
Copy link
Member

Cool, thanks!

I reviewed this PR and it seems correct.

But also this PR should only affect tier 3 targets, so... yolo!

@workingjubilee
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 9, 2024

📌 Commit c059eb7 has been approved by workingjubilee

It is now in the queue for this repository.

@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 Nov 9, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 9, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 9, 2024
@bors
Auto merge of rust-lang#132800 - matthiaskrgr:rollup-c1kkj56, r=matth…
8044557 
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#132552 (Add v9, v8plus, and leoncasa target feature to sparc and use v8plus in create_object_file)
 - rust-lang#132745 (pointee_info_at: fix logic for recursing into enums)
 - rust-lang#132777 (try_question_mark_nop: update test for LLVM 20)
 - rust-lang#132785 (rustc_target: more target string fixes for LLVM 20)
 - rust-lang#132794 (Use a separate dir for r-a builds consistently in helix config)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b9d4ef1 into rust-lang:master Nov 9, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 9, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 9, 2024
@rust-timer
Unrolled build for rust-lang#132552
6a14d06 
Rollup merge of rust-lang#132552 - taiki-e:sparc-target-feature, r=workingjubilee

Add v9, v8plus, and leoncasa target feature to sparc and use v8plus in create_object_file

This adds the following three unstable target features:

- `v9`: SPARC-V9 instructions ([LLVM definition][sparc-v9])
  - Relevant to rust-lang#131222 (comment)
  - Relevant to rust-lang#132472 (comment)
  - This is also needed to implement taiki-e/atomic-maybe-uninit#31 (depends on inline assembly support) more robustly.
- `v8plus`: SPARC-V8+ ABI ([LLVM definition][sparc-v8plus])
  - This is added in LLVM 20. In LLVM 19 and older, it is emulated to work the same way as LLVM in each LLVM version.
  - See rust-lang#132585 (comment) for more.
- `leoncasa`: CASA instruction[^1] of LEON3 and LEON4 processors ([LLVM definition][sparc-leoncasa], LLVM feature name: `hasleoncasa`)
  - This is needed to implement taiki-e/atomic-maybe-uninit#31 (depends on inline assembly support) more robustly.

[^1]: Atomic CAS instruction

[sparc-v9]: https://github.com/llvm/llvm-project/blob/f5e4ffaa49254706ad6fa209de8aec28e20f0041/llvm/lib/Target/Sparc/Sparc.td#L37-L39
[sparc-v8plus]: https://github.com/llvm/llvm-project/blob/f5e4ffaa49254706ad6fa209de8aec28e20f0041/llvm/lib/Target/Sparc/Sparc.td#L37-L39
[sparc-leoncasa]: https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/Sparc/LeonFeatures.td#L32-L37
@taiki-e taiki-e deleted the sparc-target-feature branch November 9, 2024 16:04
@taiki-e taiki-e mentioned this pull request Nov 10, 2024
Closed
mati865 pushed a commit to mati865/rust that referenced this pull request Nov 12, 2024
@matthiaskrgr @mati865
Rollup merge of rust-lang#132552 - taiki-e:sparc-target-feature, r=wo…
60a8436 
…rkingjubilee

Add v9, v8plus, and leoncasa target feature to sparc and use v8plus in create_object_file

This adds the following three unstable target features:

- `v9`: SPARC-V9 instructions ([LLVM definition][sparc-v9])
  - Relevant to rust-lang#131222 (comment)
  - Relevant to rust-lang#132472 (comment)
  - This is also needed to implement taiki-e/atomic-maybe-uninit#31 (depends on inline assembly support) more robustly.
- `v8plus`: SPARC-V8+ ABI ([LLVM definition][sparc-v8plus])
  - This is added in LLVM 20. In LLVM 19 and older, it is emulated to work the same way as LLVM in each LLVM version.
  - See rust-lang#132585 (comment) for more.
- `leoncasa`: CASA instruction[^1] of LEON3 and LEON4 processors ([LLVM definition][sparc-leoncasa], LLVM feature name: `hasleoncasa`)
  - This is needed to implement taiki-e/atomic-maybe-uninit#31 (depends on inline assembly support) more robustly.

[^1]: Atomic CAS instruction

[sparc-v9]: https://github.com/llvm/llvm-project/blob/f5e4ffaa49254706ad6fa209de8aec28e20f0041/llvm/lib/Target/Sparc/Sparc.td#L37-L39
[sparc-v8plus]: https://github.com/llvm/llvm-project/blob/f5e4ffaa49254706ad6fa209de8aec28e20f0041/llvm/lib/Target/Sparc/Sparc.td#L37-L39
[sparc-leoncasa]: https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/Sparc/LeonFeatures.td#L32-L37
mati865 pushed a commit to mati865/rust that referenced this pull request Nov 12, 2024
@bors @mati865
Auto merge of rust-lang#132800 - matthiaskrgr:rollup-c1kkj56, r=matth…
a201416 
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#132552 (Add v9, v8plus, and leoncasa target feature to sparc and use v8plus in create_object_file)
 - rust-lang#132745 (pointee_info_at: fix logic for recursing into enums)
 - rust-lang#132777 (try_question_mark_nop: update test for LLVM 20)
 - rust-lang#132785 (rustc_target: more target string fixes for LLVM 20)
 - rust-lang#132794 (Use a separate dir for r-a builds consistently in helix config)

r? `@ghost`
`@rustbot` modify labels: rollup
@durin42 durin42 mentioned this pull request Nov 12, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@workingjubilee workingjubilee workingjubilee left review comments

Assignees

@workingjubilee workingjubilee

Labels
A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. O-SPARC Target: SPARC processors S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.84.0
Development

Successfully merging this pull request may close these issues.

Our 32-bit Sparc handling isn't quite correct yet
6 participants
@taiki-e @glaubitz @koachan @bors @rustbot @workingjubilee