Skip to content

cg_LLVM: Stop needing an alloca for volatile loads#157127

Open
scottmcm wants to merge 1 commit into
rust-lang:mainfrom
scottmcm:tweak-layout-of-alternative
Open

cg_LLVM: Stop needing an alloca for volatile loads#157127
scottmcm wants to merge 1 commit into
rust-lang:mainfrom
scottmcm:tweak-layout-of-alternative

Conversation

@scottmcm

@scottmcm scottmcm commented May 29, 2026

Copy link
Copy Markdown
Member

This ended up also being reimplementing it to not use load of the llvm_type, since without doing that everything blew up horribly. cc the zulip conversation #t-opsem > Defining volatile splitting @ 💬.

And while I'm here, improve the tests to check that the unaligned ones are actually unaligned, since unaligned_volatile_load::<u8> doesn't actually test anything.

r? @nikic
cc @RalfJung
MCP tracking issue #153250

@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 May 29, 2026
@scottmcm scottmcm added the A-codegen Area: Code generation label May 29, 2026
Comment thread compiler/rustc_codegen_llvm/src/intrinsic.rs Outdated
Comment thread compiler/rustc_codegen_llvm/src/intrinsic.rs Outdated
@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels May 31, 2026
@scottmcm scottmcm force-pushed the tweak-layout-of-alternative branch from ff8746f to 9ff8e99 Compare June 3, 2026 05:18
@rustbot

rustbot commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

The GCC codegen subtree was changed

cc @antoyo, @GuillaumeGomez

@rustbot

This comment has been minimized.

Comment thread compiler/rustc_codegen_gcc/src/builder.rs Outdated
@rust-log-analyzer

This comment has been minimized.

@scottmcm scottmcm force-pushed the tweak-layout-of-alternative branch from 9ff8e99 to d82afbd Compare June 3, 2026 05:27
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 3, 2026
@rust-log-analyzer

This comment has been minimized.

@scottmcm scottmcm 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-review Status: Awaiting review from the assignee but also interested parties. labels Jun 3, 2026
@scottmcm scottmcm force-pushed the tweak-layout-of-alternative branch from d82afbd to b5c38a6 Compare June 6, 2026 20:31
@rust-log-analyzer

This comment has been minimized.

@scottmcm scottmcm force-pushed the tweak-layout-of-alternative branch from b5c38a6 to d844e5f Compare June 6, 2026 20:49

fn volatile_load(&mut self, ty: Type<'gcc>, ptr: RValue<'gcc>) -> RValue<'gcc> {
fn volatile_load(&mut self, ty: Type<'gcc>, ptr: RValue<'gcc>, _: Align) -> RValue<'gcc> {
// FIXME(antoyo): set alignment.

@scottmcm scottmcm Jun 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

annot: it wasn't obvious to me how to get this to work -- maybe that's why it was already a FIXME -- so I ended up just moving the FIXME from the volatile_load intrinsic to the volatile_load builder method, which doesn't make anything worse at least.

View changes since the review

@rust-log-analyzer

This comment has been minimized.

@scottmcm

Copy link
Copy Markdown
Member Author

Sorry for taking a bazillion iterations on this. CI is happy now, so I think this is finally
@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 18, 2026
@rust-bors

This comment has been minimized.

And while I'm here, improve the tests to check that the unaligned ones are actually unaligned, since `unaligned_volatile_load::<u8>` doesn't actually test anything.
@scottmcm scottmcm force-pushed the tweak-layout-of-alternative branch from 9032be2 to 9b188dd Compare June 26, 2026 04:04
@rustbot

rustbot commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main 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.

@nikic

nikic commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

@bors r+

I would not be surprised if this breaks someones embedded code because they expected a volatile load of a struct to be split in some specific way. I guess we'll find out...

@rust-bors

rust-bors Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 9b188dd has been approved by nikic

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 1. This pull request will be tested once the tree is reopened.

@rust-bors rust-bors Bot 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 Jun 26, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jun 26, 2026
…ve, r=nikic

cg_LLVM: Stop needing an alloca for volatile loads

This ended up also being reimplementing it to not use `load` of the `llvm_type`, since without doing that everything blew up horribly.  cc the zulip conversation [#t-opsem > Defining volatile splitting @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/136281-t-opsem/topic/Defining.20volatile.20splitting/near/597451615).

And while I'm here, improve the tests to check that the unaligned ones are actually unaligned, since `unaligned_volatile_load::<u8>` doesn't actually test anything.

r? @nikic
cc @RalfJung
MCP tracking issue rust-lang#153250
rust-bors Bot pushed a commit that referenced this pull request Jun 26, 2026
…uwer

Rollup of 4 pull requests

Successful merges:

 - #157127 (cg_LLVM: Stop needing an alloca for volatile loads)
 - #158244 (Attribute docs `deprecated` , `warn`, `allow`, `cfg`, `deny`, and `forbid` )
 - #158399 (std: truncate thread names on NetBSD)
 - #158430 (Guard clone suggestion against empty obligation errors)
rust-bors Bot pushed a commit that referenced this pull request Jun 26, 2026
…uwer

Rollup of 4 pull requests

Successful merges:

 - #157127 (cg_LLVM: Stop needing an alloca for volatile loads)
 - #158244 (Attribute docs `deprecated` , `warn`, `allow`, `cfg`, `deny`, and `forbid` )
 - #158399 (std: truncate thread names on NetBSD)
 - #158430 (Guard clone suggestion against empty obligation errors)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jun 26, 2026
…ve, r=nikic

cg_LLVM: Stop needing an alloca for volatile loads

This ended up also being reimplementing it to not use `load` of the `llvm_type`, since without doing that everything blew up horribly.  cc the zulip conversation [#t-opsem > Defining volatile splitting @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/136281-t-opsem/topic/Defining.20volatile.20splitting/near/597451615).

And while I'm here, improve the tests to check that the unaligned ones are actually unaligned, since `unaligned_volatile_load::<u8>` doesn't actually test anything.

r? @nikic
cc @RalfJung
MCP tracking issue rust-lang#153250
rust-bors Bot pushed a commit that referenced this pull request Jun 26, 2026
…uwer

Rollup of 6 pull requests

Successful merges:

 - #158360 (Various borrowck cleanups and param_env/opaque_types_defined_by query simplifications for typeck children)
 - #157127 (cg_LLVM: Stop needing an alloca for volatile loads)
 - #158244 (Attribute docs `deprecated` , `warn`, `allow`, `cfg`, `deny`, and `forbid` )
 - #158355 (Fixup the refactoring errors in #156246)
 - #158399 (std: truncate thread names on NetBSD)
 - #158430 (Guard clone suggestion against empty obligation errors)
rust-bors Bot pushed a commit that referenced this pull request Jun 26, 2026
…uwer

Rollup of 6 pull requests

Successful merges:

 - #158360 (Various borrowck cleanups and param_env/opaque_types_defined_by query simplifications for typeck children)
 - #157127 (cg_LLVM: Stop needing an alloca for volatile loads)
 - #158244 (Attribute docs `deprecated` , `warn`, `allow`, `cfg`, `deny`, and `forbid` )
 - #158355 (Fixup the refactoring errors in #156246)
 - #158399 (std: truncate thread names on NetBSD)
 - #158430 (Guard clone suggestion against empty obligation errors)
rust-bors Bot pushed a commit that referenced this pull request Jun 26, 2026
…uwer

Rollup of 6 pull requests

Successful merges:

 - #158360 (Various borrowck cleanups and param_env/opaque_types_defined_by query simplifications for typeck children)
 - #157127 (cg_LLVM: Stop needing an alloca for volatile loads)
 - #158244 (Attribute docs `deprecated` , `warn`, `allow`, `cfg`, `deny`, and `forbid` )
 - #158355 (Fixup the refactoring errors in #156246)
 - #158399 (std: truncate thread names on NetBSD)
 - #158430 (Guard clone suggestion against empty obligation errors)
rust-bors Bot pushed a commit that referenced this pull request Jun 26, 2026
…uwer

Rollup of 6 pull requests

Successful merges:

 - #158360 (Various borrowck cleanups and param_env/opaque_types_defined_by query simplifications for typeck children)
 - #157127 (cg_LLVM: Stop needing an alloca for volatile loads)
 - #158244 (Attribute docs `deprecated` , `warn`, `allow`, `cfg`, `deny`, and `forbid` )
 - #158355 (Fixup the refactoring errors in #156246)
 - #158399 (std: truncate thread names on NetBSD)
 - #158430 (Guard clone suggestion against empty obligation errors)
rust-bors Bot pushed a commit that referenced this pull request Jun 26, 2026
…uwer

Rollup of 6 pull requests

Successful merges:

 - #158360 (Various borrowck cleanups and param_env/opaque_types_defined_by query simplifications for typeck children)
 - #157127 (cg_LLVM: Stop needing an alloca for volatile loads)
 - #158244 (Attribute docs `deprecated` , `warn`, `allow`, `cfg`, `deny`, and `forbid` )
 - #158355 (Fixup the refactoring errors in #156246)
 - #158399 (std: truncate thread names on NetBSD)
 - #158430 (Guard clone suggestion against empty obligation errors)
rust-bors Bot pushed a commit that referenced this pull request Jun 26, 2026
…uwer

Rollup of 6 pull requests

Successful merges:

 - #158360 (Various borrowck cleanups and param_env/opaque_types_defined_by query simplifications for typeck children)
 - #157127 (cg_LLVM: Stop needing an alloca for volatile loads)
 - #158244 (Attribute docs `deprecated` , `warn`, `allow`, `cfg`, `deny`, and `forbid` )
 - #158355 (Fixup the refactoring errors in #156246)
 - #158399 (std: truncate thread names on NetBSD)
 - #158430 (Guard clone suggestion against empty obligation errors)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jun 26, 2026
…ve, r=nikic

cg_LLVM: Stop needing an alloca for volatile loads

This ended up also being reimplementing it to not use `load` of the `llvm_type`, since without doing that everything blew up horribly.  cc the zulip conversation [#t-opsem > Defining volatile splitting @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/136281-t-opsem/topic/Defining.20volatile.20splitting/near/597451615).

And while I'm here, improve the tests to check that the unaligned ones are actually unaligned, since `unaligned_volatile_load::<u8>` doesn't actually test anything.

r? @nikic
cc @RalfJung
MCP tracking issue rust-lang#153250
rust-bors Bot pushed a commit that referenced this pull request Jun 26, 2026
…uwer

Rollup of 7 pull requests

Successful merges:

 - #158360 (Various borrowck cleanups and param_env/opaque_types_defined_by query simplifications for typeck children)
 - #157127 (cg_LLVM: Stop needing an alloca for volatile loads)
 - #158244 (Attribute docs `deprecated` , `warn`, `allow`, `cfg`, `deny`, and `forbid` )
 - #158355 (Fixup the refactoring errors in #156246)
 - #158361 (Move `check_ffi_pure` into the attribute parser)
 - #158399 (std: truncate thread names on NetBSD)
 - #158430 (Guard clone suggestion against empty obligation errors)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-codegen Area: Code generation 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.

6 participants