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

Add inline assembly support for m68k #109989

Merged
merged 1 commit into from
Apr 13, 2023
Merged

Add inline assembly support for m68k #109989

merged 1 commit into from
Apr 13, 2023

Conversation

ids1024
Copy link
Contributor

@ids1024 ids1024 commented Apr 6, 2023

I believe this should be correct, to the extent I understand the logic around inline assembly. M68k is fairly straightforward here, other than having separate address registers.

@rustbot
Copy link
Collaborator

rustbot commented Apr 6, 2023

r? @petrochenkov

(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 6, 2023
@ids1024 ids1024 marked this pull request as draft April 6, 2023 02:41
@ids1024
Copy link
Contributor Author

ids1024 commented Apr 6, 2023

Apparently rustc_codegen_gcc will need changes too for CI to pass.

@ids1024 ids1024 marked this pull request as ready for review April 6, 2023 03:48
@rustbot
Copy link
Collaborator

rustbot commented Apr 6, 2023

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

@petrochenkov
Copy link
Contributor

cc @Amanieu
@bors r+

@bors
Copy link
Contributor

bors commented Apr 6, 2023

📌 Commit ba1d7b9fe5a5bc21cff2c4866c4a518f1b668eba has been approved by petrochenkov

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 Apr 6, 2023
@Amanieu
Copy link
Member

Amanieu commented Apr 6, 2023

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 6, 2023
compiler/rustc_codegen_llvm/src/asm.rs Outdated Show resolved Hide resolved
compiler/rustc_target/src/asm/m68k.rs Outdated Show resolved Hide resolved
compiler/rustc_target/src/asm/m68k.rs Outdated Show resolved Hide resolved
compiler/rustc_target/src/asm/m68k.rs Outdated Show resolved Hide resolved
compiler/rustc_target/src/asm/m68k.rs Outdated Show resolved Hide resolved
@Amanieu
Copy link
Member

Amanieu commented Apr 6, 2023

Also please add a test in tests/assembly/asm to ensure that:

  • LLVM properly handles all input types for each register class.
  • The register name is rendered correctly for all input types/classes.

@petrochenkov
Copy link
Contributor

r? @Amanieu

@rustbot rustbot assigned Amanieu and unassigned petrochenkov Apr 6, 2023
@ids1024 ids1024 force-pushed the m68k-asm branch 2 times, most recently from 57d278c to 6a151c9 Compare April 11, 2023 03:23
@ids1024 ids1024 force-pushed the m68k-asm branch 2 times, most recently from 4c3e2db to a0de8e1 Compare April 12, 2023 02:06
@@ -244,6 +244,7 @@ impl<'ll, 'tcx> AsmBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
InlineAsmArch::Msp430 => {
constraints.push("~{sr}".to_string());
}
InlineAsmArch::M68k => {}
Copy link
Member

Choose a reason for hiding this comment

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

This should clobber ccr. Also document this in the unstable book under the preserves_flags section.

@Amanieu
Copy link
Member

Amanieu commented Apr 13, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Apr 13, 2023

📌 Commit 2ac8dee has been approved by Amanieu

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 13, 2023
@bors
Copy link
Contributor

bors commented Apr 13, 2023

⌛ Testing commit 2ac8dee with merge 1870bbdaba6f3e8202139f5ce939223adb875e9c...

@bors
Copy link
Contributor

bors commented Apr 13, 2023

💔 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 Apr 13, 2023
@Amanieu
Copy link
Member

Amanieu commented Apr 13, 2023

@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 Apr 13, 2023
@bors
Copy link
Contributor

bors commented Apr 13, 2023

⌛ Testing commit 2ac8dee with merge e14b81f...

@bors
Copy link
Contributor

bors commented Apr 13, 2023

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing e14b81f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 13, 2023
@bors bors merged commit e14b81f into rust-lang:master Apr 13, 2023
@rustbot rustbot added this to the 1.70.0 milestone Apr 13, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e14b81f): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.6% [0.4%, 0.9%] 4
Regressions ❌
(secondary)
2.3% [2.3%, 2.3%] 1
Improvements ✅
(primary)
-1.3% [-1.8%, -0.8%] 2
Improvements ✅
(secondary)
-2.2% [-2.2%, -2.2%] 1
All ❌✅ (primary) -0.0% [-1.8%, 0.9%] 6

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.8% [0.5%, 1.4%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.8% [0.5%, 1.4%] 3

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
curl: (35) OpenSSL SSL_connect: Connection reset by peer in connection to ci-artifacts.rust-lang.org:443 

error: failed to download llvm from ci

    help: old builds get deleted after a certain time
    help: if trying to compile an old commit of rustc, disable `download-ci-llvm` in config.toml:

    [llvm]
    download-ci-llvm = false
    
cat: /tmp/toolstate/toolstates.json: No such file or directory

@Amanieu
Copy link
Member

Amanieu commented Apr 13, 2023

@bors retry

@Amanieu
Copy link
Member

Amanieu commented Apr 13, 2023

Oh wait this is already merged.

antoyo pushed a commit to antoyo/rust that referenced this pull request Jun 19, 2023
Add inline assembly support for m68k

I believe this should be correct, to the extent I understand the logic around inline assembly. M68k is fairly straightforward here, other than having separate address registers.
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.

7 participants