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

rustc_codegen_ssa: cleanup AtomicOrdering #97369

Merged
merged 2 commits into from
May 26, 2022

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented May 24, 2022

  • Remove unused NotAtomic ordering.
  • Rename Monotonic to Relaxed - a Rust specific name.
  • Derive copy and clone.

@rust-highfive
Copy link
Collaborator

Some changes occured to rustc_codegen_gcc

cc @antoyo

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

r? @michaelwoerister

(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 May 24, 2022
tmiasko added 2 commits May 25, 2022 10:34
* Remove unused `NotAtomic` ordering.
* Rename `Monotonic` to `Relaxed` - a Rust specific name.
@tmiasko tmiasko force-pushed the codgen-ssa-atomic-ordering branch from 7b26185 to ef83e68 Compare May 25, 2022 08:38
@michaelwoerister
Copy link
Member

To understand what's going on here: On the Rust side we use the term "relaxed" where LLVM uses the term "monotonic" and this PR makes the Rust side of things consistently use "relaxed"?

@tmiasko
Copy link
Contributor Author

tmiasko commented May 25, 2022

To understand what's going on here: On the Rust side we use the term "relaxed" where LLVM uses the term "monotonic" and this PR makes the Rust side of things consistently use "relaxed"?

Yes, that's right.

@michaelwoerister
Copy link
Member

OK, that makes sense to me 👍
Thanks, @tmiasko!
@bors r+

@bors
Copy link
Contributor

bors commented May 25, 2022

📌 Commit ef83e68 has been approved by michaelwoerister

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

bors commented May 25, 2022

⌛ Testing commit ef83e68 with merge b8f6eaa5cad987c1350d03d46d3b8b827c0e8021...

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
error: make failed
status: exit code: 2
command: "make"
--- stdout -------------------------------
make[1]: Entering directory '/d/a/rust/rust/src/test/run-make-fulldeps/stdin-non-utf8'
cp non-utf8 /d/a/rust/rust/build/x86_64-pc-windows-gnu/test/run-make-fulldeps/stdin-non-utf8/stdin-non-utf8/non-utf.rs
cat /d/a/rust/rust/build/x86_64-pc-windows-gnu/test/run-make-fulldeps/stdin-non-utf8/stdin-non-utf8/non-utf.rs | PATH="/d/a/rust/rust/build/x86_64-pc-windows-gnu/test/run-make-fulldeps/stdin-non-utf8/stdin-non-utf8:D:\a\rust\rust\build\x86_64-pc-windows-gnu\stage2\bin:/d/a/rust/rust/build/x86_64-pc-windows-gnu/stage0-bootstrap-tools/x86_64-pc-windows-gnu/release/deps:/d/a/rust/rust/build/x86_64-pc-windows-gnu/stage0/bin:/d/a/rust/rust/ninja:/d/a/rust/rust/mingw64/bin:/c/hostedtoolcache/windows/Python/3.10.4/x64/Scripts:/c/hostedtoolcache/windows/Python/3.10.4/x64:/usr/bin:/d/a/rust/rust/sccache:/c/Program Files/MongoDB/Server/5.0/bin:/c/aliyun-cli:/c/vcpkg:/c/cf-cli:/c/Program Files (x86)/NSIS:/c/tools/zstd:/c/Program Files/Mercurial:/c/hostedtoolcache/windows/stack/2.7.5/x64:/c/cabal/bin:/c/ghcup/bin:/c/tools/ghc-9.2.2/bin:/c/Program Files/dotnet:/c/mysql/bin:/c/Program Files/R/R-4.2.0/bin/x64:/c/SeleniumWebDrivers/GeckoDriver:/c/Program Files (x86)/sbt/bin:/c/Program Files (x86)/GitHub CLI:/c/Program Files/Git/bin:/c/Program Files (x86)/pipx_bin:/c/hostedtoolcache/windows/go/1.17.10/x64/bin:/c/hostedtoolcache/windows/Python/3.7.9/x64/Scripts:/c/hostedtoolcache/windows/Python/3.7.9/x64:/c/hostedtoolcache/windows/Ruby/2.5.9/x64/bin:/c/tools/kotlinc/bin:/c/hostedtoolcache/windows/Java_Temurin-Hotspot_jdk/8.0.332-9/x64/bin:/c/npm/prefix:/c/Program Files (x86)/Microsoft SDKs/Azure/CLI2/wbin:/c/ProgramData/kind:/c/Program Files/Eclipse Foundation/jdk-8.0.302.8-hotspot/bin:/c/Windows/system32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0:/c/Windows/System32/OpenSSH:/c/ProgramData/Chocolatey/bin:/c/Program Files/Docker:/c/Program Files/PowerShell/7:/c/Program Files/Microsoft/Web Platform Installer:/c/Program Files/dotnet:/c/Program Files/Microsoft SQL Server/130/Tools/Binn:/c/Program Files/Microsoft SQL Server/Client SDK/ODBC/170/Tools/Binn:/c/Program Files (x86)/Windows Kits/10/Windows Performance Toolkit:/c/Program Files (x86)/Microsoft SQL Server/110/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/120/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/130/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/140/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/150/DTS/Binn:/c/Program Files/nodejs:/c/Program Files/OpenSSL/bin:/c/Strawberry/c/bin:/c/Strawberry/perl/site/bin:/c/Strawberry/perl/bin:/c/ProgramData/chocolatey/lib/pulumi/tools/Pulumi/bin:/c/Program Files/TortoiseSVN/bin:/c/Program Files/CMake/bin:/c/ProgramData/chocolatey/lib/maven/apache-maven-3.8.5/bin:/c/Program Files/Microsoft Service Fabric/bin/Fabric/Fabric.Code:/c/Program Files/Microsoft SDKs/Service Fabric/Tools/ServiceFabricLocalClusterManager:/c/Program Files/Git/cmd:/c/Program Files/Git/mingw64/bin:/c/Program Files/Git/usr/bin:/c/Program Files/GitHub CLI:/c/tools/php:/c/Program Files (x86)/sbt/bin:/c/SeleniumWebDrivers/ChromeDriver:/c/SeleniumWebDrivers/EdgeDriver:/c/Program Files/Amazon/AWSCLIV2:/c/Program Files/Amazon/SessionManagerPlugin/bin:/c/Program Files/Amazon/AWSSAMCLI/bin:/c/Program Files (x86)/Google/Cloud SDK/google-cloud-sdk/bin:/c/Program Files (x86)/Microsoft BizTalk Server:/c/Program Files/LLVM/bin:/c/Users/runneradmin/.dotnet/tools:/c/Users/runneradmin/.cargo/bin:/c/Users/runneradmin/AppData/Local/Microsoft/WindowsApps" 'D:\a\rust\rust\build\x86_64-pc-windows-gnu\stage2\bin\rustc.exe' --out-dir /d/a/rust/rust/build/x86_64-pc-windows-gnu/test/run-make-fulldeps/stdin-non-utf8/stdin-non-utf8 -L /d/a/rust/rust/build/x86_64-pc-windows-gnu/test/run-make-fulldeps/stdin-non-utf8/stdin-non-utf8  - 2>&1 \
 | "D:\a\rust\rust/src/etc/cat-and-grep.sh" "error: couldn't read from stdin, as it did not contain valid UTF-8"
[[[ begin stdout ]]]
I/O error: operation failed to complete synchronously

[[[ end stdout ]]]
Error: cannot match: error: couldn't read from stdin, as it did not contain valid UTF-8
make[1]: Leaving directory '/d/a/rust/rust/src/test/run-make-fulldeps/stdin-non-utf8'
--- stderr -------------------------------
--- stderr -------------------------------
make[1]: *** [Makefile:5: all] Error 1



failures:
failures:
    [run-make] src/test\run-make-fulldeps\stdin-non-utf8

Some tests failed in compiletest suite=run-make-fulldeps mode=run-make host=x86_64-pc-windows-gnu target=x86_64-pc-windows-gnu
test result: FAILED. 192 passed; 1 failed; 37 ignored; 0 measured; 0 filtered out; finished in 79.06s

Build completed unsuccessfully in 1:23:20
make: *** [Makefile:78: ci-mingw-subset-1] Error 1

@bors
Copy link
Contributor

bors commented May 25, 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 May 25, 2022
@tmiasko
Copy link
Contributor Author

tmiasko commented May 25, 2022

@bors retry

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

bors commented May 26, 2022

⌛ Testing commit ef83e68 with merge 99c4758...

@bors
Copy link
Contributor

bors commented May 26, 2022

☀️ Test successful - checks-actions
Approved by: michaelwoerister
Pushing 99c4758 to master...

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

Finished benchmarking commit (99c4758): comparison url.

Instruction count

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

Max RSS (memory usage)

Results
  • Primary benchmarks: mixed results
  • Secondary benchmarks: 🎉 relevant improvement found
mean1 max count2
Regressions 😿
(primary)
6.2% 6.2% 1
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
-5.7% -5.7% 1
Improvements 🎉
(secondary)
-1.0% -1.0% 1
All 😿🎉 (primary) 0.3% 6.2% 2

Cycles

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)
2.9% 2.9% 1
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) 2.9% 2.9% 1

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

  2. number of relevant changes 2

@tmiasko tmiasko deleted the codgen-ssa-atomic-ordering branch May 26, 2022 07:27
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.

8 participants