Skip to content

Conversation

folkertdev
Copy link
Contributor

fixes #146761

Provide a better MemoryEffects to LLVM when an inline assembly block specifies readonly but not pure. That means that the assembly block may not perform any writes, but that there still may be side effects from its instructions.

I haven't been able to find a case yet where this actually matters, though. So the test checks that the right attribute is applied, but the generated assembly is equivalent to not specifying readonly at all.

r? @nikic
cc @Amanieu

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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 Sep 19, 2025
@rust-log-analyzer

This comment has been minimized.

@joshtriplett
Copy link
Member

LGTM (but not r+ing because you requested a specific reviewer).

}

// CHECK-LABEL: @readonly_not_pure
// CHECK: call i32 asm
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to check that the attributes are actually emitted on the right call. Something like call i32 asm {{.*}} #[[ATTR:\d+]] here and attributes #[[ATTR]] below should do it.

@rustbot
Copy link
Collaborator

rustbot commented Sep 21, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@folkertdev
Copy link
Contributor Author

@bors r=nikic,joshtriplett

@bors
Copy link
Collaborator

bors commented Sep 21, 2025

📌 Commit 3565b06 has been approved by nikic,joshtriplett

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 Sep 21, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 22, 2025
…kic,joshtriplett

emit attribute for readonly non-pure inline assembly

fixes rust-lang#146761

Provide a better `MemoryEffects` to LLVM when an inline assembly block specifies `readonly` but not `pure`. That means that the assembly block may not perform any writes, but that there still may be side effects from its instructions.

I haven't been able to find a case yet where this actually matters, though. So the test checks that the right attribute is applied, but the generated assembly is equivalent to not specifying `readonly` at all.

r? `@nikic`
cc `@Amanieu`
bors added a commit that referenced this pull request Sep 22, 2025
Rollup of 8 pull requests

Successful merges:

 - #146317 (Add panic=immediate-abort)
 - #146397 (std_detect on Darwin AArch64: update features)
 - #146594 (bootstrap: Don't force -static for musl targets in cc-rs)
 - #146791 (emit attribute for readonly non-pure inline assembly)
 - #146831 (Support ctr and lr as clobber-only registers in PowerPC inline assembly)
 - #146838 (Introduce "wrapper" helpers to rustdoc)
 - #146846 (btree InternalNode::new safety comments)
 - #146858 (Make mips64el-unknown-linux-muslabi64 link dynamically)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 22, 2025
…kic,joshtriplett

emit attribute for readonly non-pure inline assembly

fixes rust-lang#146761

Provide a better `MemoryEffects` to LLVM when an inline assembly block specifies `readonly` but not `pure`. That means that the assembly block may not perform any writes, but that there still may be side effects from its instructions.

I haven't been able to find a case yet where this actually matters, though. So the test checks that the right attribute is applied, but the generated assembly is equivalent to not specifying `readonly` at all.

r? ``@nikic``
cc ``@Amanieu``
bors added a commit that referenced this pull request Sep 22, 2025
Rollup of 10 pull requests

Successful merges:

 - #145411 (regression test for Cow<[u8]> layout)
 - #146317 (Add panic=immediate-abort)
 - #146397 (std_detect on Darwin AArch64: update features)
 - #146594 (bootstrap: Don't force -static for musl targets in cc-rs)
 - #146652 (Port `feature` to the new attribute system)
 - #146791 (emit attribute for readonly non-pure inline assembly)
 - #146831 (Support ctr and lr as clobber-only registers in PowerPC inline assembly)
 - #146838 (Introduce "wrapper" helpers to rustdoc)
 - #146846 (btree InternalNode::new safety comments)
 - #146858 (Make mips64el-unknown-linux-muslabi64 link dynamically)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 22, 2025
…kic,joshtriplett

emit attribute for readonly non-pure inline assembly

fixes rust-lang#146761

Provide a better `MemoryEffects` to LLVM when an inline assembly block specifies `readonly` but not `pure`. That means that the assembly block may not perform any writes, but that there still may be side effects from its instructions.

I haven't been able to find a case yet where this actually matters, though. So the test checks that the right attribute is applied, but the generated assembly is equivalent to not specifying `readonly` at all.

r? ```@nikic```
cc ```@Amanieu```
bors added a commit that referenced this pull request Sep 22, 2025
Rollup of 9 pull requests

Successful merges:

 - #145411 (regression test for Cow<[u8]> layout)
 - #146317 (Add panic=immediate-abort)
 - #146397 (std_detect on Darwin AArch64: update features)
 - #146594 (bootstrap: Don't force -static for musl targets in cc-rs)
 - #146791 (emit attribute for readonly non-pure inline assembly)
 - #146831 (Support ctr and lr as clobber-only registers in PowerPC inline assembly)
 - #146838 (Introduce "wrapper" helpers to rustdoc)
 - #146846 (btree InternalNode::new safety comments)
 - #146858 (Make mips64el-unknown-linux-muslabi64 link dynamically)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Sep 22, 2025
Rollup of 9 pull requests

Successful merges:

 - #145411 (regression test for Cow<[u8]> layout)
 - #146317 (Add panic=immediate-abort)
 - #146397 (std_detect on Darwin AArch64: update features)
 - #146594 (bootstrap: Don't force -static for musl targets in cc-rs)
 - #146791 (emit attribute for readonly non-pure inline assembly)
 - #146831 (Support ctr and lr as clobber-only registers in PowerPC inline assembly)
 - #146838 (Introduce "wrapper" helpers to rustdoc)
 - #146846 (btree InternalNode::new safety comments)
 - #146858 (Make mips64el-unknown-linux-muslabi64 link dynamically)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Sep 22, 2025
Rollup of 9 pull requests

Successful merges:

 - #145411 (regression test for Cow<[u8]> layout)
 - #146397 (std_detect on Darwin AArch64: update features)
 - #146791 (emit attribute for readonly non-pure inline assembly)
 - #146831 (Support ctr and lr as clobber-only registers in PowerPC inline assembly)
 - #146838 (Introduce "wrapper" helpers to rustdoc)
 - #146845 (Add self-profile events for target-machine creation)
 - #146846 (btree InternalNode::new safety comments)
 - #146858 (Make mips64el-unknown-linux-muslabi64 link dynamically)
 - #146878 (assert_unsafe_precondition: fix some incorrect check_language_ub)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 40db498 into rust-lang:master Sep 22, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 22, 2025
rust-timer added a commit that referenced this pull request Sep 22, 2025
Rollup merge of #146791 - folkertdev:readonly-not-pure, r=nikic,joshtriplett

emit attribute for readonly non-pure inline assembly

fixes #146761

Provide a better `MemoryEffects` to LLVM when an inline assembly block specifies `readonly` but not `pure`. That means that the assembly block may not perform any writes, but that there still may be side effects from its instructions.

I haven't been able to find a case yet where this actually matters, though. So the test checks that the right attribute is applied, but the generated assembly is equivalent to not specifying `readonly` at all.

r? ````@nikic````
cc ````@Amanieu````
Muscraft pushed a commit to Muscraft/rust that referenced this pull request Sep 24, 2025
…kic,joshtriplett

emit attribute for readonly non-pure inline assembly

fixes rust-lang#146761

Provide a better `MemoryEffects` to LLVM when an inline assembly block specifies `readonly` but not `pure`. That means that the assembly block may not perform any writes, but that there still may be side effects from its instructions.

I haven't been able to find a case yet where this actually matters, though. So the test checks that the right attribute is applied, but the generated assembly is equivalent to not specifying `readonly` at all.

r? ````@nikic````
cc ````@Amanieu````
Muscraft pushed a commit to Muscraft/rust that referenced this pull request Sep 24, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang#145411 (regression test for Cow<[u8]> layout)
 - rust-lang#146397 (std_detect on Darwin AArch64: update features)
 - rust-lang#146791 (emit attribute for readonly non-pure inline assembly)
 - rust-lang#146831 (Support ctr and lr as clobber-only registers in PowerPC inline assembly)
 - rust-lang#146838 (Introduce "wrapper" helpers to rustdoc)
 - rust-lang#146845 (Add self-profile events for target-machine creation)
 - rust-lang#146846 (btree InternalNode::new safety comments)
 - rust-lang#146858 (Make mips64el-unknown-linux-muslabi64 link dynamically)
 - rust-lang#146878 (assert_unsafe_precondition: fix some incorrect check_language_ub)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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.

Emit memory attribute for readonly non-pure inline asm
6 participants