Skip to content

emit attribute for readonly non-pure inline assembly#146791

Merged
bors merged 1 commit intorust-lang:masterfrom
folkertdev:readonly-not-pure
Sep 22, 2025
Merged

emit attribute for readonly non-pure inline assembly#146791
bors merged 1 commit intorust-lang:masterfrom
folkertdev:readonly-not-pure

Conversation

@folkertdev
Copy link
Contributor

@folkertdev folkertdev commented Sep 19, 2025

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.

Sign in to view
@joshtriplett
Copy link
Member

joshtriplett commented Sep 21, 2025

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

nikic
nikic approved these changes Sep 21, 2025
View reviewed changes
tests/codegen-llvm/asm/readonly-not-pure.rs Outdated
}

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

@nikic nikic Sep 21, 2025

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

folkertdev commented Sep 21, 2025

@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
@Zalathar
Rollup merge of rust-lang#146791 - folkertdev:readonly-not-pure, r=ni…
1cb5a4d 
…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`
@Zalathar Zalathar mentioned this pull request Sep 22, 2025
Closed
bors added a commit that referenced this pull request Sep 22, 2025
@bors
Auto merge of #146871 - Zalathar:rollup-e2pedcw, r=Zalathar
ce32c9b 
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
@Zalathar
Rollup merge of rust-lang#146791 - folkertdev:readonly-not-pure, r=ni…
a25ee5a 
…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``
@Zalathar Zalathar mentioned this pull request Sep 22, 2025
Closed
bors added a commit that referenced this pull request Sep 22, 2025
@bors
Auto merge of #146873 - Zalathar:rollup-u87iupw, r=Zalathar
702aba1 
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
@Zalathar
Rollup merge of rust-lang#146791 - folkertdev:readonly-not-pure, r=ni…
7d7a2c4 
…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```
@Zalathar Zalathar mentioned this pull request Sep 22, 2025
Closed
bors added a commit that referenced this pull request Sep 22, 2025
@bors
Auto merge of #146876 - Zalathar:rollup-hfvkrpi, r=Zalathar
d3da100 
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
@bors
Auto merge of #146876 - Zalathar:rollup-hfvkrpi, r=Zalathar
ac11cdb 
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
@Zalathar Zalathar mentioned this pull request Sep 22, 2025
Merged
bors added a commit that referenced this pull request Sep 22, 2025
@bors
Auto merge of #146879 - Zalathar:rollup-vm97j8b, r=Zalathar
29005cb 
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
@rust-timer
Unrolled build for #146791
a3c8c3a 
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
@Zalathar @Muscraft
Rollup merge of rust-lang#146791 - folkertdev:readonly-not-pure, r=ni…
e3a1a71 
…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
@bors @Muscraft
Auto merge of rust-lang#146879 - Zalathar:rollup-vm97j8b, r=Zalathar
d68687c 
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
Kobzol pushed a commit to Kobzol/rustc_codegen_gcc that referenced this pull request Dec 21, 2025
@bors
Auto merge of #146879 - Zalathar:rollup-vm97j8b, r=Zalathar
8524e7a 
Rollup of 9 pull requests

Successful merges:

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

Reviewers

@nikic nikic nikic approved these changes

Assignees

@nikic nikic

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

Milestone

1.92.0

Development

Successfully merging this pull request may close these issues.

Emit memory attribute for readonly non-pure inline asm

6 participants

@folkertdev @rust-log-analyzer @joshtriplett @rustbot @bors @nikic