Fix RemoveUnstructuredLoopExits when an exiting edge jumps out multiple levels of loops. #6668
Conversation
dneto0
force-pushed
the
issue-163
branch
2 times, most recently
from
e62783e
to
55338e7
Compare
|
yeah, I need to fix this. I thought I was running with ASAN but I wasn't. |
Indeed pictures are great. I've added ASCII art depictions of the CFGs for the two reduced test cases. The "regression" case is a mess, and doesn't provide extra insight. |
|
The failures are weird; I can't explain them by this change. An error is caused by this warning (with warnings-as-error): The source at that spot has a pragma that appears to guard the warning. I'll try a no-change amend commit to kick off the bots again. |
…le levels of loops Before doing any major surgery on an exit from loop L, ensure that if an exit edge from L goes to block X, then X is in L's parent loop or no loop at all. Add test cases: - a reduced test case where the exiting block does not dominate its own loop latch. - a reduced test case where the exiting block is the latch for its own loop. This reproduces the assert triggered by the original HLSL. - the original HLSL that triggered this bug fix. - the intermediate module from the original HLSL, taken just before the attempt to remove unstructured loop exits.
|
Trying again by rebasing against latest main. |
|
The Windows build failures here seem to be related to a VM image update that occurred last night. Our builds work fine on VM image 20240514.3.0, and fail on 20240603.1.0. Given that I'm fairly confident this PR isn't related to the issues I'm going to bypass the merge checks to merge the PR. |
We do not want this change to stick around forever but we need it to work around the broken GitHub runner images: actions/runner-images#10004 Related microsoft#6668
…t multiple levels of loops. (microsoft#6668)" This reverts commit 8206fbd. Reason for revert: since landing this, new asserts/crashes have been found.
This PR contains two changes: 1) Moves a pragma to disable a warning, which seems to be required by the new compiler. 2) Adds a preprocessor define to workaround the crashes caused by the runner image mismatching C++ runtime versions. The second change we will want to revert once the runner images are fixed. The issue tracking the runner images is: actions/runner-images#10004 Related #6668
…t multiple levels of loops. (microsoft#6668)" This reverts commit 8206fbd. Reason for revert: since landing this, new asserts/crashes have been found.
This PR contains two changes: 1) Moves a pragma to disable a warning, which seems to be required by the new compiler. 2) Adds a preprocessor define to workaround the crashes caused by the runner image mismatching C++ runtime versions. The second change we will want to revert once the runner images are fixed. The issue tracking the runner images is: actions/runner-images#10004 Related microsoft#6668 (cherry picked from commit 0b9acdb)
Before doing any major surgery on an exit from loop L, ensure that if an exit edge from L goes to block X, then X is in L's parent loop or no loop at all.
Add test cases: