Skip to content

Fix intermittent debug failures with Debug build#7369

Merged
jkwak-work merged 5 commits intoshader-slang:masterfrom
jkwak-work:test/fix-intermittent-debug-failures
Jun 12, 2025
Merged

Fix intermittent debug failures with Debug build#7369
jkwak-work merged 5 commits intoshader-slang:masterfrom
jkwak-work:test/fix-intermittent-debug-failures

Conversation

@jkwak-work
Copy link
Copy Markdown
Collaborator

@jkwak-work jkwak-work commented Jun 7, 2025

This PR replaces enable/disable style C function calls with C++ RAII style code.

In debug build, when an assertion failed in between enable and disable functions, an exception is thrown and the disable function is not called. RAII style code is safer for an exception

@jkwak-work jkwak-work self-assigned this Jun 7, 2025
@jkwak-work jkwak-work added the pr: non-breaking PRs without breaking changes label Jun 7, 2025
@shader-slang shader-slang deleted a comment from slangbot Jun 7, 2025
@jkwak-work jkwak-work force-pushed the test/fix-intermittent-debug-failures branch from e4e4487 to c99f277 Compare June 7, 2025 07:29
@jkwak-work
Copy link
Copy Markdown
Collaborator Author

jkwak-work commented Jun 9, 2025

With this fix,

  • "windows/debug" shows <1% failure rate (0 failures out of 107 tries)
  • "windows/release" shows <1% failure rate (0 failures out of 150 tries)
  • "linux/release/gcc" shows <2% failure rate (2 failures out of 121 tries)
  • "macos/release/clang" shows 8% failure rate (4 failures out of 50 tries).

When failures are observed on "linux/release/gcc", the following test failed.

tests/language-server/robustness-7.slang

When failures are observed on "macos/release/clang", the following tests failed,

tests/language-server/robustness-8.slang
tests/language-server/vector-member.slang

Note that without this fix, the failure rate of linux and macos wasn't 0%.
On "linux/release/gcc", the failure rate was 2%, which is about the same.
On "macos/release/clang", the failure rate was 4%, which was lower.

You can find more information here.

@jkwak-work jkwak-work force-pushed the test/fix-intermittent-debug-failures branch from c99f277 to 6702935 Compare June 9, 2025 15:45
@jkwak-work jkwak-work marked this pull request as ready for review June 9, 2025 15:45
@jkwak-work jkwak-work requested a review from a team as a code owner June 9, 2025 15:45
@jkwak-work jkwak-work changed the title [DNI] Fix intermittent debug failures Fix intermittent debug failures with Debug build Jun 9, 2025
@jkwak-work
Copy link
Copy Markdown
Collaborator Author

closes #7333

@gtong-nv
Copy link
Copy Markdown
Contributor

gtong-nv commented Jun 9, 2025

I agree that the RAII style is better. But I don't understand how it helps the intermittent assert...
Looks like for some autodiff code, there are some inst. that are not able to pass IR validation so we disable that check first, and then we reenable it afterwards. For example,

            disableIRValidationAtInsert();
            diffBuilder->addBackwardDerivativePrimalContextDecoration(callInst, intermediateVar);
            enableIRValidationAtInsert();

It's always disable first, and then enable again.
If something happens in between, we should in the _enableIRValidationAtInsert = false state, and no validation would be done, hence no more assert would be trigger. Seems like we are seeing the opposite.

Copy link
Copy Markdown
Contributor

@gtong-nv gtong-nv left a comment

Choose a reason for hiding this comment

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

LGTM

@jkwak-work jkwak-work enabled auto-merge (squash) June 12, 2025 00:14
@jkwak-work jkwak-work merged commit 7dad68f into shader-slang:master Jun 12, 2025
17 checks passed
@jkwak-work jkwak-work deleted the test/fix-intermittent-debug-failures branch June 12, 2025 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: non-breaking PRs without breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants