Skip to content

Allow Rvalue::Repeat to return true in rvalue_creates_operand too #143720

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

scottmcm
Copy link
Member

@scottmcm scottmcm commented Jul 10, 2025

The conversation in #143502 (comment) made be realize how easy this is to handle, since the only possibilty is ZSTs -- everything else ends up with the destination being LocalKind::Memory and thus doesn't call codegen_rvalue_operand at all.

This gets us perilously close to a world where rvalue_creates_operand only ever returns true. (See #143860 for more.)

@rustbot
Copy link
Collaborator

rustbot commented Jul 10, 2025

r? @lcnr

rustbot has assigned @lcnr.
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

@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 Jul 10, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 10, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

The conversation in 143502 made be realize how easy this is to handle, since the only possibilty is ZSTs -- everything else ends up with the destination being `LocalKind::Memory` and thus doesn't call `codegen_rvalue_operand` at all.

This gets us perilously close to a world where `rvalue_creates_operand` only ever returns true.  I'll try out such a world next :)
@scottmcm scottmcm force-pushed the rvalue-always-operand branch from e6eb401 to 45e04b8 Compare July 10, 2025 07:03
bors added a commit that referenced this pull request Jul 13, 2025
Let `codegen_transmute_operand` just handle everything

When combined with #143720, this means `rvalue_creates_operand` can just return `true` for *every* `Rvalue`.  (A future PR could consider removing it, though just letting it optimize out is fine for now.)

It's nicer anyway, IMHO, because it avoids needing the layout checks to be consistent in the two places, and thus is an overall reduction in code.  Plus it's a more helpful building block when used in other places this way.

(TBH, it probably would have been better to have it this way the whole time, but I clearly didn't understand `rvalue_creates_operand` when I originally wrote #109843.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants