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

[AMDGPU] Allow hoisting of V_CMP feeding into SI_IF and SI_IF_BREAK #71401

Closed
wants to merge 1 commit into from
Closed

[AMDGPU] Allow hoisting of V_CMP feeding into SI_IF and SI_IF_BREAK #71401

wants to merge 1 commit into from

Conversation

jayfoad
Copy link
Contributor

@jayfoad jayfoad commented Nov 6, 2023

The lowering of SI_IF and SI_IF_BREAK includes an AND with EXEC (unless
it is provably not required), so compares feeding into these pseudo ops
should be just as hoistable as compares feeding into explicit ANDs with
EXEC.

The lowering of SI_IF and SI_IF_BREAK includes an AND with EXEC (unless
it is provably not required), so compares feeding into these pseudo ops
should be just as hoistable as compares feeding into explicit ANDs with
EXEC.
@jayfoad jayfoad requested a review from perlfu November 6, 2023 14:31
Copy link
Contributor

@perlfu perlfu left a comment

Choose a reason for hiding this comment

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

This looks reasonable.

I guess we could end up with "worse" code gen. for SI_IF_BREAK in some edge cases.
If the V_CMP is hoisted then an extra S_AND will be added.
Although the S_AND will probably have lower latency than a V_CMP?

case AMDGPU::S_AND_SAVEEXEC_B32:
case AMDGPU::S_AND_SAVEEXEC_B64:
break;
case AMDGPU::SI_IF:
case AMDGPU::SI_IF_BREAK:
if (Use.getOperandNo() != 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably this branch is specific to SI_IF_BREAK?
Could you add extra comment to explain what operand 1 is.

@jayfoad jayfoad closed this by deleting the head repository Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants