Fix simplify if-else #6077
Merged
Fix simplify if-else #6077
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
csyonghe
reviewed
Jan 14, 2025
| // we can get rid of the entire conditional branch and replace it | ||
| // with a jump into the after block. | ||
| if (auto termInst = as<IRUnconditionalBranch>(ifElseInst->getTrueBlock()->getTerminator())) | ||
| IRUnconditionalBranch* termInst; | ||
| if (termInst = as<IRUnconditionalBranch>(ifElseInst->getTrueBlock()->getTerminator())) |
There was a problem hiding this comment.
style: fold the initialization to the declaration site of termInst, and just do if (termInst & termInst->getTargetBlock() != ifElseInst->getAfterBlock())
cheneym2
force-pushed
the
acheney/ifelse2
branch
from
d31a774 to
c5ea59f
Compare
The if-else optimization observes that at if at least one
true/false block is merely an unconditional jump to the
after block, that the whole if-else can be replaced with
a jump to the after block. But it's important to copy
the phi arguments from the aforementioned unconditional
jump, rather than what is present in the 'true' block,
since the 'true' block might actually just be the after
block itself.
Below, the ifElse() would be replaced with an unconditional
jump to block %39, but with the `phi` arguments copied from
the branch to %29, which is an unrelated block.
ifElse(%38, %39, %40, %39)
block %40:
unconditionalBranch(%39)
block %39:
unconditionalBranch(%29, 0 : Float)
block %29(
[nameHint("ret")]
param %ret : Float):
Fixes issue shader-slang#5972
cheneym2
force-pushed
the
acheney/ifelse2
branch
from
c5ea59f to
d2f55a3
Compare
|
/format |
|
🌈 Formatted, please merge the changes from this PR |
Format code for PR shader-slang#6077
csyonghe
approved these changes
Jan 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The if-else optimization observes that at if at least one true/false block is merely an unconditional jump to the after block, and both are "trivial", that the whole if-else can be replaced with a jump to the after block. But it's important to copy the phi arguments from the aforementioned unconditional jump, rather than what is present in the 'true' block, since the 'true' block might actually just be the after block itself.
Below, the ifElse() would be replaced with an unconditional jump to block %39, but with the
phiarguments copied from the branch to %29, which is an unrelated block.ifElse(%38, %39, %40, %39)
block %40:
unconditionalBranch(%39)
block %39:
unconditionalBranch(%29, 0 : Float)
block %29(
[nameHint("ret")]
param %ret : Float):
Fixes issue #5972