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

use 128 cache align for aarch64 #97414

Merged
merged 1 commit into from
Jun 2, 2022
Merged

Conversation

LYF1999
Copy link
Contributor

@LYF1999 LYF1999 commented May 26, 2022

the cache line size of m1 mac is 128.
so use align(128) for m1 mac

here is sysctl -a hw machdep.cpu output on m1 mac

hw.ncpu: 10
hw.byteorder: 1234
hw.memsize: 68719476736
hw.activecpu: 10
hw.perflevel0.physicalcpu: 8
hw.perflevel0.physicalcpu_max: 8
hw.perflevel0.logicalcpu: 8
hw.perflevel0.logicalcpu_max: 8
hw.perflevel0.l1icachesize: 196608
hw.perflevel0.l1dcachesize: 131072
hw.perflevel0.l2cachesize: 12582912
hw.perflevel0.cpusperl2: 4
hw.perflevel1.physicalcpu: 2
hw.perflevel1.physicalcpu_max: 2
hw.perflevel1.logicalcpu: 2
hw.perflevel1.logicalcpu_max: 2
hw.perflevel1.l1icachesize: 131072
hw.perflevel1.l1dcachesize: 65536
hw.perflevel1.l2cachesize: 4194304
hw.perflevel1.cpusperl2: 2
hw.optional.arm.FEAT_FlagM: 1
hw.optional.arm.FEAT_FlagM2: 1
hw.optional.arm.FEAT_FHM: 1
hw.optional.arm.FEAT_DotProd: 1
hw.optional.arm.FEAT_SHA3: 1
hw.optional.arm.FEAT_RDM: 1
hw.optional.arm.FEAT_LSE: 1
hw.optional.arm.FEAT_SHA256: 1
hw.optional.arm.FEAT_SHA512: 1
hw.optional.arm.FEAT_SHA1: 1
hw.optional.arm.FEAT_AES: 1
hw.optional.arm.FEAT_PMULL: 1
hw.optional.arm.FEAT_SPECRES: 0
hw.optional.arm.FEAT_SB: 1
hw.optional.arm.FEAT_FRINTTS: 1
hw.optional.arm.FEAT_LRCPC: 1
hw.optional.arm.FEAT_LRCPC2: 1
hw.optional.arm.FEAT_FCMA: 1
hw.optional.arm.FEAT_JSCVT: 1
hw.optional.arm.FEAT_PAuth: 1
hw.optional.arm.FEAT_PAuth2: 0
hw.optional.arm.FEAT_FPAC: 0
hw.optional.arm.FEAT_DPB: 1
hw.optional.arm.FEAT_DPB2: 1
hw.optional.arm.FEAT_BF16: 0
hw.optional.arm.FEAT_I8MM: 0
hw.optional.arm.FEAT_ECV: 1
hw.optional.arm.FEAT_LSE2: 1
hw.optional.arm.FEAT_CSV2: 1
hw.optional.arm.FEAT_CSV3: 1
hw.optional.arm.FEAT_FP16: 1
hw.optional.arm.FEAT_SSBS: 1
hw.optional.arm.FEAT_BTI: 0
hw.optional.floatingpoint: 1
hw.optional.neon: 1
hw.optional.neon_hpfp: 1
hw.optional.neon_fp16: 1
hw.optional.armv8_1_atomics: 1
hw.optional.armv8_2_fhm: 1
hw.optional.armv8_2_sha512: 1
hw.optional.armv8_2_sha3: 1
hw.optional.armv8_3_compnum: 1
hw.optional.watchpoint: 4
hw.optional.breakpoint: 6
hw.optional.armv8_crc32: 1
hw.optional.armv8_gpi: 1
hw.optional.AdvSIMD: 1
hw.optional.AdvSIMD_HPFPCvt: 1
hw.optional.ucnormal_mem: 1
hw.optional.arm64: 1
hw.features.allows_security_research: 0
hw.physicalcpu: 10
hw.physicalcpu_max: 10
hw.logicalcpu: 10
hw.logicalcpu_max: 10
hw.cputype: 16777228
hw.cpusubtype: 2
hw.cpu64bit_capable: 1
hw.cpufamily: 458787763
hw.cpusubfamily: 5
hw.cacheconfig: 10 1 2 0 0 0 0 0 0 0
hw.cachesize: 3373957120 65536 4194304 0 0 0 0 0 0 0
hw.pagesize: 16384
hw.pagesize32: 16384
hw.cachelinesize: 128
hw.l1icachesize: 131072
hw.l1dcachesize: 65536
hw.l2cachesize: 4194304
hw.tbfrequency: 24000000
hw.packages: 1
hw.osenvironment:
hw.ephemeral_storage: 0
hw.use_recovery_securityd: 0
hw.use_kernelmanagerd: 1
hw.serialdebugmode: 0
hw.nperflevels: 2
hw.targettype: J316c
machdep.cpu.cores_per_package: 10
machdep.cpu.core_count: 10
machdep.cpu.logical_per_package: 10
machdep.cpu.thread_count: 10
machdep.cpu.brand_string: Apple M1 Max

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label May 26, 2022
@rust-highfive
Copy link
Collaborator

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rust-highfive
Copy link
Collaborator

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

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 26, 2022
@LYF1999
Copy link
Contributor Author

LYF1999 commented May 26, 2022

PTAL @Mark-Simulacrum

@Mark-Simulacrum
Copy link
Member

Based on some relatively quick searching online, it seems like a 128 cache line size is fairly common on other aarch64 implementations, so it probably makes some sense for us to just gate on aarch64, rather than macOS and aarch64 here. Ultimately this is just for performance (and mostly on parallel-compiler, which isn't enabled by default and isn't on a clear path to being enabled), so getting it wrong is not likely to be terribly important in practice.

r=me with this made more general to affect all aarch64.

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 1, 2022
@LYF1999 LYF1999 changed the title use 128 cache align for m1 mac use 128 cache align for aarch64 Jun 1, 2022
@LYF1999
Copy link
Contributor Author

LYF1999 commented Jun 1, 2022

Hi, @Mark-Simulacrum , why does it mostly affect on parallel-compiler? it's CacheAligned in std lib mpsc, not the one in rust compiler.

@LYF1999
Copy link
Contributor Author

LYF1999 commented Jun 1, 2022

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 1, 2022
@Mark-Simulacrum
Copy link
Member

Oh, well, I probably just misread. Regardless this seems okay now, thanks!

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Jun 1, 2022

📌 Commit 1446bce has been approved by Mark-Simulacrum

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

bors commented Jun 2, 2022

⌛ Testing commit 1446bce with merge 12ebf275fe69e41584ff2cbf8aff8a0297572bcb...

@bors
Copy link
Contributor

bors commented Jun 2, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 2, 2022
@rust-log-analyzer
Copy link
Collaborator

The job i686-msvc-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
test [rustdoc] src/test\rustdoc\where.rs ... ok

failures:

---- [rustdoc] src/test\rustdoc\primitive\no_std.rs stdout ----
thread '[rustdoc] src/test\rustdoc\primitive\no_std.rs' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 5, kind: PermissionDenied, message: "Access is denied." }', src\tools\compiletest\src\runtest.rs:2262:34


failures:
    [rustdoc] src/test\rustdoc\primitive\no_std.rs
    [rustdoc] src/test\rustdoc\primitive\no_std.rs

test result: FAILED. 516 passed; 1 failed; 8 ignored; 0 measured; 0 filtered out; finished in 79.78s

Some tests failed in compiletest suite=rustdoc mode=rustdoc host=i686-pc-windows-msvc target=i686-pc-windows-msvc
Build completed unsuccessfully in 0:41:24
make: *** [Makefile:70: ci-subset-1] Error 1

@Mark-Simulacrum
Copy link
Member

@bors retry msvc access denied in rustdoc tests

Seems likely to be spurious.

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

bors commented Jun 2, 2022

⌛ Testing commit 1446bce with merge fb19760...

@bors
Copy link
Contributor

bors commented Jun 2, 2022

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing fb19760 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 2, 2022
@bors bors merged commit fb19760 into rust-lang:master Jun 2, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 2, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (fb19760): comparison url.

Instruction count

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

Max RSS (memory usage)

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: 🎉 relevant improvements found
mean1 max count2
Regressions 😿
(primary)
2.5% 2.5% 1
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
-2.2% -2.4% 2
All 😿🎉 (primary) 2.5% 2.5% 1

Cycles

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

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change

  2. number of relevant changes

@LYF1999 LYF1999 deleted the yf/cachealign branch October 28, 2022 16:23
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-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants