Skip to content

Comments

Added default target cpu to --print target-cpus output and updated docs#110876

Merged
bors merged 4 commits intorust-lang:masterfrom
mj10021:issue-110647-fix
May 5, 2023
Merged

Added default target cpu to --print target-cpus output and updated docs#110876
bors merged 4 commits intorust-lang:masterfrom
mj10021:issue-110647-fix

Conversation

@mj10021
Copy link
Contributor

@mj10021 mj10021 commented Apr 27, 2023

Added default target cpu info as requested in issue #110647 and noted the new output in the documentation

@rustbot
Copy link
Collaborator

rustbot commented Apr 27, 2023

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

@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. labels Apr 27, 2023
@compiler-errors
Copy link
Contributor

compiler-errors commented Apr 27, 2023

Can you show how this actually renders, so the reviewer doesn't need to test it themself?

@mj10021
Copy link
Contributor Author

mj10021 commented Apr 27, 2023

Can you show how this actually renders, so the reviewer doesn't need to test it themself?

Sure thing. Running rustc --print target-cpus shows:

Default cpu for this target:
    x86-64
Available CPUs for this target:
    native         - Select the CPU of the current host (currently skylake).
    alderlake     
    amdfam10      
    athlon
   ...

and running rustc --print target-cpus --target i686-unknown-linux-gnu shows:

Default cpu for this target:
    pentium4
Available CPUs for this target:
    alderlake     
    amdfam10      
    athlon 
   ...

@mj10021
Copy link
Contributor Author

mj10021 commented Apr 27, 2023

Can you show how this actually renders, so the reviewer doesn't need to test it themself?

Sure thing. Running rustc --print target-cpus shows:

Default cpu for this target:
    x86-64
Available CPUs for this target:
    native         - Select the CPU of the current host (currently skylake).
    alderlake     
    amdfam10      
    athlon
   ...

and running rustc --print target-cpus --target i686-unknown-linux-gnu shows:

Default cpu for this target:
    pentium4
Available CPUs for this target:
    alderlake     
    amdfam10      
    athlon 
   ...

Changing cpu to CPU to match the previous text

@compiler-errors
Copy link
Contributor

compiler-errors commented Apr 27, 2023

I think the comment #110647 (comment) was asking for this to be rendered inline, as a note after the default CPU flavor. I agree with that comment. How hard is it to change the behavior to act like that instead?

@mj10021
Copy link
Contributor Author

mj10021 commented Apr 27, 2023

I think the comment #110647 (comment) was asking for this to be rendered inline, as a note after the default CPU flavor. I agree with that comment. How hard is it to change the behavior to act like that instead?

Got it, working on this now, I think I just need to pass the default target cpu into unsafe { llvm::LLVMRustPrintTargetCPUs(tm) }

@mj10021
Copy link
Contributor Author

mj10021 commented Apr 28, 2023

This is the new output:

rustc --print target-cpus 

Available CPUs for this target:
    native         - Select the CPU of the current host (currently skylake).
    alderlake
    amdfam10
    athlon
    athlon-4
    athlon-fx
    athlon-mp
    athlon-tbird
    athlon-xp
    athlon64
    athlon64-sse3
    atom
    barcelona
    bdver1
    bdver2
    bdver3
    bdver4
    bonnell
    broadwell
    btver1
    btver2
    c3
    c3-2
    cannonlake
    cascadelake
    cooperlake
    core-avx-i
    core-avx2
    core2
    corei7
    corei7-avx
    emeraldrapids
    generic
    geode
    goldmont
    goldmont-plus
    grandridge
    graniterapids
    haswell
    i386
    i486
    i586
    i686
    icelake-client
    icelake-server
    ivybridge
    k6
    k6-2
    k6-3
    k8
    k8-sse3
    knl
    knm
    lakemont
    meteorlake
    nehalem
    nocona
    opteron
    opteron-sse3
    penryn
    pentium
    pentium-m
    pentium-mmx
    pentium2
    pentium3
    pentium3m
    pentium4
    pentium4m
    pentiumpro
    prescott
    raptorlake
    rocketlake
    sandybridge
    sapphirerapids
    sierraforest
    silvermont
    skx
    skylake
    skylake-avx512
    slm
    tigerlake
    tremont
    westmere
    winchip-c6
    winchip2
    x86-64           - this is the default target cpu
    x86-64-v2
    x86-64-v3
    x86-64-v4
    yonah
    znver1
    znver2
    znver3
    znver4

Let me know if I did something wrong with the C++, I don't have much experience there. Thanks!

@mj10021
Copy link
Contributor Author

mj10021 commented Apr 28, 2023

Maybe the text should read - this is the default target cpu for the current build target ?

@mj10021
Copy link
Contributor Author

mj10021 commented Apr 28, 2023

Maybe the text should read - this is the default target cpu for the current build target ?

also I need to capitalize CPU again lol

@mj10021
Copy link
Contributor Author

mj10021 commented Apr 30, 2023 •
edited
Loading

Updated the output to
x86-64 - This is the default target CPU for the current build target (currently x86_64-unknown-linux-gnu).
and changed .expect() to .unwrap_or_else()

b-naber
b-naber reviewed May 4, 2023
View reviewed changes
compiler/rustc_codegen_llvm/src/llvm_util.rs Outdated
Copy link
Contributor

@b-naber b-naber May 4, 2023

Choose a reason for hiding this comment

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

nit: Maybe a SAFETY comment here explaining why this is ok (had to look at the docs for CString to check that it is)?

@mj10021 mj10021 force-pushed the issue-110647-fix branch from 4f76695 to cb74cd5 Compare May 5, 2023 00:54
@b-naber
Copy link
Contributor

b-naber commented May 5, 2023

Thanks. @bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 5, 2023

📌 Commit f239cd6 has been approved by b-naber

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 May 5, 2023
@Dylan-DPC Dylan-DPC mentioned this pull request May 5, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 5, 2023
@bors
Auto merge of rust-lang#111248 - Dylan-DPC:rollup-lbp0ui3, r=Dylan-DPC
4b94c23 
Rollup of 6 pull requests

Successful merges:

 - rust-lang#103056 (Fix `checked_{add,sub}_duration` incorrectly returning `None` when `other` has more than `i64::MAX` seconds)
 - rust-lang#108801 (Implement RFC 3348, `c"foo"` literals)
 - rust-lang#110773 (Reduce MIR dump file count for MIR-opt tests)
 - rust-lang#110876 (Added default target cpu to `--print target-cpus` output and updated docs)
 - rust-lang#111068 (Improve check-cfg implementation)
 - rust-lang#111238 (btree_map: `Cursor{,Mut}::peek_prev` must agree)

Failed merges:

 - rust-lang#110694 (Implement builtin # syntax and use it for offset_of!(...))

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 65702bf into rust-lang:master May 5, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 5, 2023
@workingjubilee workingjubilee mentioned this pull request Jul 22, 2023
Closed
@mj10021 mj10021 deleted the issue-110647-fix branch November 1, 2024 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@b-naber b-naber b-naber left review comments

Assignees

@b-naber b-naber

Labels

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.71.0

Development

Successfully merging this pull request may close these issues.

5 participants

@mj10021 @rustbot @compiler-errors @b-naber @bors