PIX: Don't seek beyond terminator instructions (value-to-declare pass… #3856
Conversation
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
microsoft#3855) Background: this pass is trying to find all dbg.value and replace them with dbg.declare. In the code being changed, the pass is trying to seek a valid location at which to insert the dbg.declare. It has to come after the value to which it applies (which isn't true of the dbg.value). So there's this little loop trying to move forward to find the right instruction before which to insert new stuff. I was expecting getNextNode to return null when there is no next node. When called on a terminator, it actually returns a non-null but malformed instruction pointer. So we have to explicitly check for terminators in this loop. This really short basic block tripped up the pass: ; <label>:274 ; preds = %.lr.ph55 %RawBufferLoad = call %dx.types.ResRet.i32 @dx.op.rawBufferLoad.i32(i32 139, %dx.types.Handle %lightBuffer_texture_structbuf, i32 %lightIndex.0, i32 28, i8 1, i32 4), !dbg !384 %275 = extractvalue %dx.types.ResRet.i32 %RawBufferLoad, 0, !dbg !384 switch i32 %275, label %288 [ i32 0, label %276 i32 1, label %280 i32 2, label %284 ], !dbg !397 I think the pass could be smarter about seeking the right insertion point for the dbg.declare. It's currently assuming that the dbg.value always succeeds the value to which it refers, but as in this case, that's not always true. But that's a project for another day. (cherry picked from commit 650de80)
|
✅ Build DirectXShaderCompiler 1.0.326 completed (commit 2e6fdcfc56 by @jeffnn) |
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.
…) (#3855)
Background: this pass is trying to find all dbg.value and replace them with dbg.declare. In the code being changed, the pass is trying to seek a valid location at which to insert the dbg.declare. It has to come after the value to which it applies (which isn't true of the dbg.value). So there's this little loop trying to move forward to find the right instruction before which to insert new stuff.
I was expecting getNextNode to return null when there is no next node. When called on a terminator, it actually returns a non-null but malformed instruction pointer. So we have to explicitly check for terminators in this loop.
This really short basic block tripped up the pass:
; :274 ; preds = %.lr.ph55
%RawBufferLoad = call %dx.types.ResRet.i32 @dx.op.rawBufferLoad.i32(i32 139, %dx.types.Handle %lightBuffer_texture_structbuf, i32 %lightIndex.0, i32 28, i8 1, i32 4), !dbg !384
%275 = extractvalue %dx.types.ResRet.i32 %RawBufferLoad, 0, !dbg !384
switch i32 %275, label %288 [
i32 0, label %276
i32 1, label %280
i32 2, label %284
], !dbg !397
I think the pass could be smarter about seeking the right insertion point for the dbg.declare. It's currently assuming that the dbg.value always succeeds the value to which it refers, but as in this case, that's not always true. But that's a project for another day.
(cherry picked from commit 650de80)