Skip to content
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

Allow emitting more enums as SSA values #138373

Open
scottmcm opened this issue Mar 11, 2025 · 1 comment
Open

Allow emitting more enums as SSA values #138373

scottmcm opened this issue Mar 11, 2025 · 1 comment
Labels
A-codegen Area: Code generation C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

scottmcm commented Mar 11, 2025

In https://github.com/rust-lang/rust/pull/138157/files#diff-ec1143edac0931158bf79d598432eb162f53400086d30bfe1ba5266559831174R17 you can see that, because it's in a loop (I think [ed: wrongly, apparently]), the Option<u32> ends up getting spilled to an alloca despite it having ScalarPair ABI.

However it's only used in the same basic block, with the use after the initialization. Thus ideally we'd be able to just emit it as SSA values directly, rather than needing the read/write to the alloca.

(The full rule is probably a dominance check, but I don't know if the full one is worth doing, vs a simple approximation that catches common cases.)

@scottmcm scottmcm added the A-codegen Area: Code generation label Mar 11, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 11, 2025
@tmiasko
Copy link
Contributor

tmiasko commented Mar 11, 2025

I think this one is placed in an alloca due to ProjectionElem::Downcast.

Definitions in a loops should be recognized as SSA just fine already.

@tmiasko tmiasko removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 11, 2025
@scottmcm scottmcm changed the title Allow emitting SSA values in loops where possible Allow emitting more enums as SSA values Mar 12, 2025
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants