Skip to content

[MachineBB] Make sure there are successors in terminatorIsComputedGoto. #151342

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

Merged
merged 2 commits into from
Jul 31, 2025

Conversation

fhahn
Copy link
Contributor

@fhahn fhahn commented Jul 30, 2025

Currently terminatorIsComputedGoto will return for blocks with a indirect branch terminator and no successor. If there are no successor, the terminator is likely not a computed goto, return false in that case.

Note that this is currently NFC, as the only use checks it only if there are successors, but it will be needed in
#150911.

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

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

Technically, you can have a computed goto with no legal destinations... which doesn't matter in the context of taildup, but the name might trick someone into believing the check is actually reliable.

If we can't easily make the check reliable, maybe rename it?

fhahn added 2 commits July 30, 2025 21:13
Currently terminatorIsComputedGoto will return for blocks with a
indirect branch terminator and no successor. If there are no successor,
the terminator is likely not a computed goto, return false in that case.

Note that this is currently NFC, as the only use checks it only if there
are successors, but it will be needed in
llvm#150911.
@fhahn fhahn force-pushed the mbb-terminatorIsComputedGoto branch from 92fb44f to 533a414 Compare July 30, 2025 20:33
@fhahn
Copy link
Contributor Author

fhahn commented Jul 30, 2025

Technically, you can have a computed goto with no legal destinations... which doesn't matter in the context of taildup, but the name might trick someone into believing the check is actually reliable.

If we can't easily make the check reliable, maybe rename it?

How about terminatorIsComputedGotoWithSuccessors? Updated to the name for now

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

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

LGTM

@fhahn fhahn merged commit 69f3ea0 into llvm:main Jul 31, 2025
9 checks passed
@fhahn fhahn deleted the mbb-terminatorIsComputedGoto branch July 31, 2025 16:52
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jul 31, 2025
…ComputedGoto. (#151342)

Currently terminatorIsComputedGoto will return for blocks with a
indirect branch terminator and no successor. If there are no successor,
the terminator is likely not a computed goto, return false in that case.

Note that this is currently NFC, as the only use checks it only if there
are successors, but it will be needed in
llvm/llvm-project#150911.

PR: llvm/llvm-project#151342
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants