Skip to content

Add tests for raw-dylib with vectorcall, and fix vectorcall code generation - #99476

Merged
bors merged 1 commit into
rust-lang:masterfrom
dpaoliello:rawdylibvectorcall
Aug 1, 2022
Merged

Add tests for raw-dylib with vectorcall, and fix vectorcall code generation#99476
bors merged 1 commit into
rust-lang:masterfrom
dpaoliello:rawdylibvectorcall

Conversation

@dpaoliello

@dpaoliellodpaoliello commented Jul 19, 2022

Copy link
Copy Markdown
Contributor
  • Adds tests for using raw-dylib (Tracking issue for RFC 2627: #[link(kind="raw-dylib")] #58713) with vectorcall.
  • Fixed code generation for vectorcall (parameters have to be marked with InReg, just like fastcall).
  • Enabled running the raw-dylibfastcall tests when using MSVC (since I had to add support in the test for running MSVC-only tests since GCC doesn't support vectorcall).

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

rust-highfive commented Jul 19, 2022

Copy link
Copy Markdown
Contributor

r? @cjgillot

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 19, 2022
luqmana
luqmana reviewed Jul 20, 2022
View reviewed changes
Comment threadcompiler/rustc_target/src/abi/call/mod.rs Outdated
Comment threadcompiler/rustc_metadata/src/native_libs.rs Outdated
Comment threadsrc/test/run-make/raw-dylib-alt-calling-convention/lib.rs Outdated
@dpaoliello

dpaoliello commented Jul 22, 2022

Copy link
Copy Markdown
ContributorAuthor

r? @luqmana

@luqmana

luqmana commented Jul 22, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

LGTM but don't have r+ bits, so r? @petrochenkov or @michaelwoerister who've reviewed previous raw-dylib PRs

@bors

bors commented Jul 26, 2022

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #98989) made this pull request unmergeable. Please resolve the merge conflicts.

@dpaoliellodpaoliello mentioned this pull request Jul 26, 2022
Open
17 tasks
@dpaoliello

dpaoliello commented Jul 27, 2022

Copy link
Copy Markdown
ContributorAuthor

r? @michaelwoerister

@michaelwoerister

michaelwoerister commented Aug 1, 2022

Copy link
Copy Markdown
Member

Thanks for the PR, @dpaoliello! I cross-checked the code of X86_32ABIInfo::shouldPrimitiveUseInReg() in Clang and it looks like __fastcall and __vectorcall are indeed treated the same way there. The tests look good to me too.

@bors r+

@bors

bors commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit 722d67d has been approved by michaelwoerister

It is now in the queue for this repository.

@borsbors 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 Aug 1, 2022
@michaelwoerister

michaelwoerister commented Aug 1, 2022

Copy link
Copy Markdown
Member

@bors rollup=never (modifies MSVC-only run-make tests)

@bors

bors commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 722d67d with merge 6bd0054b26a250470fbce3a4f9e9a72d99be277a...

@bors

bors commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

💔 Test failed - checks-actions

@borsbors 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 Aug 1, 2022
@rust-log-analyzer

rust-log-analyzer commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

A job failed! Check out the build log: (web)(plain)

Click to see the possible cause of the failure (guessed by this bot)

@dpaoliello

dpaoliello commented Aug 1, 2022

Copy link
Copy Markdown
ContributorAuthor

@michaelwoerister looks like one of the builds timed out (dist-aarch64-apple failed with The operation was canceled after 1h 50min without any other error messages), can you please requeue this?

@wesleywiser

wesleywiser commented Aug 1, 2022

Copy link
Copy Markdown
Member

@bors retry

@borsbors 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 Aug 1, 2022
@bors

bors commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 722d67d with merge 6b92c2f647d8d9c2219c14f84225516b223d1bf7...

@rust-log-analyzer

rust-log-analyzer commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

A job failed! Check out the build log: (web)(plain)

Click to see the possible cause of the failure (guessed by this bot)

@wesleywiser

wesleywiser commented Aug 1, 2022

Copy link
Copy Markdown
Member

fatal: unable to access 'https://github.com/rust-lang/cargo.git/': Could not resolve host: github.com

@bors retry

@bors

bors commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 722d67d with merge fe33428...

@bors

bors commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

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

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Aug 1, 2022
@bors
bors merged commit fe33428 into rust-lang:masterAug 1, 2022
@rustbotrustbot added this to the 1.64.0 milestone Aug 1, 2022
@borsbors mentioned this pull request Aug 1, 2022
Merged
@rust-timer

rust-timer commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (fe33428): comparison url.

Instruction count

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

Max RSS (memory usage)

Results
  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: no relevant changes found
mean1maxcount2
Regressions 😿
(primary)		N/AN/A0
Regressions 😿
(secondary)		N/AN/A0
Improvements 🎉
(primary)		-2.4%-2.6%2
Improvements 🎉
(secondary)		N/AN/A0
All 😿🎉 (primary)-2.4%-2.6%2

Cycles

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 😿 relevant regression found
mean1maxcount2
Regressions 😿
(primary)		N/AN/A0
Regressions 😿
(secondary)		2.6%2.6%1
Improvements 🎉
(primary)		N/AN/A0
Improvements 🎉
(secondary)		N/AN/A0
All 😿🎉 (primary)N/AN/A0

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

@dpaoliello
dpaoliello deleted the rawdylibvectorcall branch August 15, 2022 17:09
@michaelwoeristermichaelwoerister mentioned this pull request Nov 14, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@luqmanaluqmanaluqmana left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@michaelwoeristermichaelwoerister

Labels

merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.64.0

Development

Successfully merging this pull request may close these issues.

10 participants

@dpaoliello@rust-highfive@luqmana@bors@michaelwoerister@rust-log-analyzer@wesleywiser@rust-timer@cjgillot@rustbot