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

Add support for MIPS VZ ISA extension #99443

Merged
merged 1 commit into from
Sep 14, 2022

Conversation

jam1garner
Copy link
Contributor

@jam1garner jam1garner commented Jul 19, 2022

Link to relevant LLVM line where virt extension is specified

This has been tested on mips-unknown-linux-musl with a target-cpu that is >= MIPS32 5 and target-features=+virt. The example was checked in a disassembler to ensure the correct assembly sequence was being generated using the virtualization instructions.

Needed additional work:

Example docs for later:

#### `mips` or `mips64`

This platform requires that `#[target_feature]` is only applied to [`unsafe`
functions][unsafe function]. This target's feature support is currently unstable
and must be enabled by `#![feature(mips_target_feature)]` ([Issue #44839])

[Issue #44839]: https://github.com/rust-lang/rust/issues/44839

Further documentation on these features can be found in the [MIPS Instruction Set
Reference Manual], or elsewhere on [mips.com].

[MIPS Instruction Set Reference Manual]: https://s3-eu-west-1.amazonaws.com/downloads-mips/documents/MD00086-2B-MIPS32BIS-AFP-6.06.pdf
[developer.arm.com]: https://www.mips.com/products/architectures/ase/

Feature        | Implicitly Enables | Description
---------------|--------------------|-------------------
`fp64`         |                    | 64-bit Floating Point
`msa`          |                    | "MIPS SIMD Architecture"
`virt`         |                    | Virtualization instructions (VZ ASE)

If the above is good I can also submit a PR for that if there's interest in documenting it while it's still unstable. Otherwise that can be dropped, I just wrote it before realizing it was possibly not a good idea.

Relevant to #44839

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 19, 2022
@rust-highfive
Copy link
Collaborator

r? @estebank

(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 Jul 19, 2022
@pnkfelix
Copy link
Member

pnkfelix commented Sep 1, 2022

@rustbot reroll

@apiraino
Copy link
Contributor

apiraino commented Sep 1, 2022

r? rust-lang/compiler

1 similar comment
@pnkfelix
Copy link
Member

pnkfelix commented Sep 1, 2022

r? rust-lang/compiler

@rust-highfive rust-highfive assigned oli-obk and unassigned estebank Sep 1, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Sep 1, 2022

r? @nagisa (looks like you know what's up from the linked issue)

@rust-highfive rust-highfive assigned nagisa and unassigned oli-obk Sep 1, 2022
@nagisa
Copy link
Member

nagisa commented Sep 3, 2022

This would benefit from a test of some sort verifying that virt is in fact an accepted feature name, etc. You mentioned verifying certain instruction sequences, so if there is a trivial test that could be encoded as a codegen/assembly test, that would be awesome, although not required.

r=me even if you decide to not pursue adding any tests.

@nagisa
Copy link
Member

nagisa commented Sep 13, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Sep 13, 2022

📌 Commit bec3a54 has been approved by nagisa

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 Sep 13, 2022
@bors
Copy link
Contributor

bors commented Sep 14, 2022

⌛ Testing commit bec3a54 with merge c97922d...

@bors
Copy link
Contributor

bors commented Sep 14, 2022

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing c97922d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 14, 2022
@bors bors merged commit c97922d into rust-lang:master Sep 14, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 14, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c97922d): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.3% [-2.3%, -2.3%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.9% [2.1%, 3.8%] 6
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-6.5% [-6.5%, -6.5%] 1
All ❌✅ (primary) - - 0

Footnotes

  1. the arithmetic mean of the percent change 2

  2. number of relevant changes 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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
Development

Successfully merging this pull request may close these issues.

10 participants