Skip phis with no incoming values when placing a nulled shadow - #3127
Open
ZuseZ4 wants to merge 1 commit into
Open
Skip phis with no incoming values when placing a nulled shadow#3127ZuseZ4 wants to merge 1 commit into
ZuseZ4 wants to merge 1 commit into
Conversation
When invertPointerM nulls the shadow of a partially-float constant it inserts an alloca, a store of the primal and the per-byte zeroing just after the value's clone, walking past any following phis so the stores do not land in the phi region. That walk stopped at a phi with no incoming values. The PHINode case of this same function creates its shadow phi with CreatePHI(shadowTy, phi->getNumIncomingValues()) and only calls addIncoming after it has recursed into invertPointerM for each incoming value. So while that recursion runs, the shadow phi genuinely has zero incoming values, the walk stops on it, and the stores are inserted above it. That leaves the block's phis no longer contiguous at its start, and the resulting function is rejected: Instruction does not dominate all uses! PHI nodes not grouped at top of basic block! function failed verification Both complaints come from the single misplacement: the cache store for the primal phi is subsequently emitted at getFirstNonPHI(), which by then precedes that phi's definition. The incoming count says nothing about whether a phi is in the phi region, so skip every phi. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or 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
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.
When invertPointerM nulls the shadow of a partially-float constant it inserts an alloca, a store of the primal and the per-byte zeroing just after the value's clone, walking past any following phis so the stores do not land in the phi region. That walk stopped at a phi with no incoming values.
The PHINode case of this same function creates its shadow phi with CreatePHI(shadowTy, phi->getNumIncomingValues()) and only calls addIncoming after it has recursed into invertPointerM for each incoming value. So while that recursion runs, the shadow phi genuinely has zero incoming values, the walk stops on it, and the stores are inserted above it.
That leaves the block's phis no longer contiguous at its start, and the resulting function is rejected:
Instruction does not dominate all uses!
PHI nodes not grouped at top of basic block!
function failed verification
Both complaints come from the single misplacement: the cache store for the primal phi is subsequently emitted at getFirstNonPHI(), which by then precedes that phi's definition.
The incoming count says nothing about whether a phi is in the phi region, so skip every phi.
fixes: #3029
closes: #3126
cc @wsmoses