Skip to content

codegen: skip stores for entirely-uninit constant aggregate fields#157797

Open
glandium wants to merge 1 commit into
rust-lang:mainfrom
glandium:uninit
Open

codegen: skip stores for entirely-uninit constant aggregate fields#157797
glandium wants to merge 1 commit into
rust-lang:mainfrom
glandium:uninit

Conversation

@glandium

@glandium glandium commented Jun 12, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

MIR GVN (since #147827) propagates MaybeUninit::uninit() as const <uninit> in aggregate constructions. Without this fix, codegen would emit a memcpy from an [N x i8] undef global for each such field, which LLVM materializes as zero-initialization.

This mirrors the existing all_bytes_uninit skip already present for Rvalue::Use (added in #147827) into the Rvalue::Aggregate field loop.

Fixes: #157743

@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 Jun 12, 2026
@rustbot

rustbot commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

r? @jackh726

rustbot has assigned @jackh726.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 73 candidates
  • Random selection from 20 candidates

@rustbot

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rustbot

This comment has been minimized.

Sign in to view
@jackh726

jackh726 commented Jun 25, 2026

Copy link
Copy Markdown
Member

Uhh...not code I'm familiar with. Let's see if a reroll helps.

@rustbot reroll

@rustbot rustbot assigned mu001999 and unassigned jackh726 Jun 25, 2026
@mu001999

mu001999 commented Jun 25, 2026

Copy link
Copy Markdown
Member

@rustbot reroll

@rustbot rustbot assigned chenyukang and unassigned mu001999 Jun 25, 2026
@chenyukang

chenyukang commented Jun 26, 2026

Copy link
Copy Markdown
Member

@rustbot reroll

@rustbot rustbot assigned oli-obk and unassigned chenyukang Jun 26, 2026
oli-obk
oli-obk requested changes Jun 26, 2026
View reviewed changes

@oli-obk oli-obk left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do you have capacity to try this out as a general fix?

View changes since this review

Comment thread compiler/rustc_codegen_ssa/src/mir/rvalue.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 Jun 26, 2026
@rustbot

rustbot commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot

rustbot commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

rustc_codegen_gcc is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_gcc instead.

cc @antoyo, @GuillaumeGomez

@rustbot rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Jun 30, 2026
@rustbot

rustbot commented Jun 30, 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.

@glandium glandium requested a review from oli-obk June 30, 2026 07:31
@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 30, 2026
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@glandium
codegen: add OperandValue::Uninit to skip stores for entirely-uninit …
4b16700 
…constants

MIR GVN propagates MaybeUninit::uninit() as `const <uninit>` in aggregate
constructions and codegen emits a memcpy from an `[N x i8] undef` global for
each such field, which LLVM materializes as zero-initialization.

Instead of special-casing `all_bytes_uninit` at each call site, add a new
`OperandValue::Uninit` variant that `eval_mir_constant_to_operand` returns for
any all-bytes-uninit constant. `store_with_flags` is a no-op for this variant,
so the fix applies uniformly across `Rvalue::Use`, `Rvalue::Repeat`, and
`Rvalue::Aggregate` without per-site checks. The variant propagates through
field extraction and transmutes, and is handled appropriately at the remaining
call sites (function arguments, discriminant reads, debug info, etc.).
@rust-log-analyzer

rust-log-analyzer commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)

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

---- net::tcp::tests::connect_timeout_error stdout ----

thread 'net::tcp::tests::connect_timeout_error' (50824) panicked at library/std/src/net/tcp/tests.rs:66:53:
called `Result::unwrap()` on an `Err` value: Os { code: 98, kind: AddrInUse, message: "Address already in use" }
stack backtrace:
   0: __rustc::rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::result::unwrap_failed

oli-obk
oli-obk reviewed Jul 6, 2026
View reviewed changes
Comment thread compiler/rustc_codegen_ssa/src/mir/block.rs
PassMode::Direct(_) => {
if let Uninit = op.val {
let ibty = bx.cx().immediate_backend_type(arg.layout);
(bx.cx().const_undef(ibty), arg.layout.align.abi, false)

@oli-obk oli-obk Jul 6, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why are some sites const_undef and some const_poison?

View changes since the review

oli-obk
oli-obk approved these changes Jul 6, 2026
View reviewed changes

@oli-obk oli-obk left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

impl lgtm now. I'm a little bit worried about ICEs due to OperandRef::immediate hitting this now, but most of the cases should be constants of types that can't be uninit, so it would get caught much earlier in the compiler

View changes since this review

@oli-obk oli-obk 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 Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obk oli-obk oli-obk approved these changes

Assignees

@oli-obk oli-obk

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Performance regression since 1.93 related to MaybeUninit being wrongfully initialized

7 participants

@glandium @rustbot @rust-log-analyzer @jackh726 @mu001999 @chenyukang @oli-obk