Pass tune-cpu to LLVM #76830

Artoria2e5 · 2020-09-17T09:41:59Z

I think this is how it should work...

See https://internals.rust-lang.org/t/expose-tune-cpu-from-llvm/13088 for the background. Or the documentation diff.

rust-highfive · 2020-09-17T09:42:03Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @lcnr (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

lcnr · 2020-09-17T09:45:41Z

Not an area I am familiar with, maybe r? @nagisa

compiler/rustc_session/src/options.rs

nagisa

This needs a dedicated test which uses the newly added -Z flag.

nagisa · 2020-09-19T00:09:24Z

src/doc/rustc/src/codegen-options/index.md

@@ -530,6 +530,19 @@ This also supports the feature `+crt-static` and `-crt-static` to control
 Each target and [`target-cpu`](#target-cpu) has a default set of enabled
 features.

+## tune-cpu


Moving this option to -Z made it unstable and not-actually-a-codegen-option. At the very least this should be annotated as unstable.

Annotated, but not moved yet since I don't know where unstable opts should be documented.

compiler/rustc_codegen_llvm/src/attributes.rs

nagisa · 2020-09-26T13:22:05Z

This overall LGTM. There are remaining compilation issues remaining before this can land and the tune-cpu documentation still needs a note that it is unstable. Please also squash.

Artoria2e5 · 2020-09-29T02:35:38Z

src/doc/rustc/src/codegen-options/index.md

+might make your code run a bit faster on the selected CPU.
+
+You can run `rustc --print target-cpus` to see the valid options to pass
+here. Additionally, `native` can be passed to use the processor of the host


Might also be a good idea to explain generic in here and target-cpu: it's basically a profile for ancient instructions tuned for modern CPU.

Probably I can explain it in target-cpu and just defer to it here.

Artoria2e5 · 2020-09-29T02:36:06Z

src/doc/rustc/src/codegen-options/index.md

+## tune-cpu
+
+This instructs `rustc` to schedule code specifically for a particular processor.
+This does not affect what instruction sets are used or the compatibility, but


should be "does not affect compatibility (features and ABI)".

compiler/rustc_codegen_llvm/src/attributes.rs

nagisa · 2020-10-03T15:59:53Z

Still blocked on a squash.

I think this is how it should work...

Artoria2e5 · 2020-10-04T23:54:41Z

Grrr https://github.bestrui.topmunity/t/set-receive-shallowupdate-on-to-allow-pushing-from-shallow-clones/2424

nagisa · 2020-10-09T21:35:03Z

@bors r+ rollup=iffy

bors · 2020-10-09T22:14:40Z

📌 Commit a35a93f has been approved by nagisa

bors · 2020-10-09T23:43:52Z

⌛ Testing commit a35a93f with merge 2df1a8d661057e1889d837fd41d16821b6acd167...

bors · 2020-10-10T03:58:52Z

💥 Test timed out

Artoria2e5 · 2020-10-13T02:04:40Z

wha--what?

nagisa · 2020-10-13T02:14:28Z

@bors retry

bors · 2020-10-13T02:49:06Z

⌛ Testing commit a35a93f with merge f54072b...

bors · 2020-10-13T04:40:54Z

☀️ Test successful - checks-actions, checks-azure
Approved by: nagisa
Pushing f54072b to master...

khyperia · 2020-10-14T07:29:05Z

Wait, can I ask why tune_cpu was added to the backend interface? It doesn't seem to be called anywhere (deleting it and running ./x.py check seems fine). Same with target_cpu, actually, it doesn't seem to be called anywhere...

Artoria2e5 · 2020-10-14T10:27:05Z

I agree that this should probably be removed. I just added it because target_cpu was there, but yeah...

rust-highfive assigned lcnr Sep 17, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 17, 2020

Artoria2e5 marked this pull request as ready for review September 17, 2020 09:42

rust-highfive assigned nagisa and unassigned lcnr Sep 17, 2020

jonas-schievink reviewed Sep 17, 2020

View reviewed changes

compiler/rustc_session/src/options.rs Outdated Show resolved Hide resolved

Artoria2e5 force-pushed the tune branch from ebfbb30 to 3283053 Compare September 17, 2020 10:22

Artoria2e5 requested a review from jonas-schievink September 17, 2020 15:51

jyn514 added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 17, 2020

nagisa reviewed Sep 19, 2020

View reviewed changes

Artoria2e5 force-pushed the tune branch from da1369e to 2ef2fef Compare September 29, 2020 02:32

Artoria2e5 commented Sep 29, 2020

View reviewed changes

cuviper reviewed Sep 29, 2020

View reviewed changes

compiler/rustc_codegen_llvm/src/attributes.rs Show resolved Hide resolved

Pass tune-cpu to LLVM

a35a93f

I think this is how it should work...

Artoria2e5 force-pushed the tune branch from cc4452c to a35a93f Compare October 4, 2020 23:55

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 Oct 9, 2020

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 10, 2020

bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 10, 2020

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 Oct 13, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 13, 2020

bors merged commit f54072b into rust-lang:master Oct 13, 2020

rustbot added this to the 1.49.0 milestone Oct 13, 2020

Artoria2e5 deleted the tune branch October 13, 2020 13:57

khyperia mentioned this pull request Oct 14, 2020

Add "current status" subtitle. EmbarkStudios/rust-gpu#56

Merged

Pass tune-cpu to LLVM #76830

Pass tune-cpu to LLVM #76830

Uh oh!

Conversation

Artoria2e5 commented Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Sep 17, 2020

Uh oh!

lcnr commented Sep 17, 2020

Uh oh!

Uh oh!

nagisa left a comment

Choose a reason for hiding this comment

Uh oh!

nagisa Sep 19, 2020

Choose a reason for hiding this comment

Uh oh!

Artoria2e5 Sep 19, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nagisa commented Sep 26, 2020

Uh oh!

Artoria2e5 Sep 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Artoria2e5 Sep 29, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nagisa commented Oct 3, 2020

Uh oh!

Artoria2e5 commented Oct 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nagisa commented Oct 9, 2020

Uh oh!

bors commented Oct 9, 2020

Uh oh!

bors commented Oct 9, 2020

Uh oh!

bors commented Oct 10, 2020

Uh oh!

Artoria2e5 commented Oct 13, 2020

Uh oh!

nagisa commented Oct 13, 2020

Uh oh!

bors commented Oct 13, 2020

Uh oh!

bors commented Oct 13, 2020

Uh oh!

khyperia commented Oct 14, 2020

Uh oh!

Artoria2e5 commented Oct 14, 2020

Uh oh!

Uh oh!

Artoria2e5 commented Sep 17, 2020 •

edited

Loading

Artoria2e5 Sep 29, 2020 •

edited

Loading

Artoria2e5 commented Oct 4, 2020 •

edited

Loading