Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report AD checkpoint contexts #5058

Merged
merged 35 commits into from
Sep 19, 2024

Conversation

venkataram-nv
Copy link
Collaborator

@venkataram-nv venkataram-nv commented Sep 11, 2024

Issue: #4997

Display checkpoint context structures for functions with non-empty contexts:

  • Enabled with the flag -report-checkpoint-intermediates
  • Report checkpoint context sides, including sizes of each field and where they were generated from
  • Includes a lot of sourceLoc copies so that the source locations can be displayed

output for reverse-loop.slang

.\reverse-loop.slang(14): note: checkpointing context of 24 bytes associated with function: 'test_simple_loop'
float test_simple_loop(float y)
      ^~~~~~~~~~~~~~~~
.\reverse-loop.slang(17): note: 20 bytes (FixedArray<float, 5> ) used to checkpoint the following item:
    float t = y;
          ^
.\reverse-loop.slang(20): note: 4 bytes (int32_t) used for a loop counter here:
    for (int i = 0; i < 3; i++)
    ^~~

output for reverse-loop-checkpoint-test.slang

.\reverse-loop-checkpoint-test.slang(51): note: checkpointing context of 20 bytes associated with function: 'computeLoop'
float3 computeLoop(float y)
       ^~~~~~~~~~~
.\reverse-loop-checkpoint-test.slang(54): note: 4 bytes (float) used to checkpoint the following item:
    float w = 0;
          ^
.\reverse-loop-checkpoint-test.slang(57): note: 12 bytes (Vector<float, 3> ) used to checkpoint the following item:
    float3 w3 = float3(0, 0, 0);
           ^~
.\reverse-loop-checkpoint-test.slang(60): note: 4 bytes (int32_t) used for a loop counter here:
    for (int i = 0; i < 8; i++)
    ^~~

@venkataram-nv venkataram-nv self-assigned this Sep 11, 2024
@venkataram-nv venkataram-nv added the pr: non-breaking PRs without breaking changes label Sep 11, 2024
@venkataram-nv venkataram-nv marked this pull request as ready for review September 16, 2024 19:56
@saipraveenb25
Copy link
Collaborator

Overall, this looks good!

However, we really need tests for this. I would suggest that we use the diagnostic sink to report these diagnostics as 'notes' (use Severity::Note in the diagnostic info) and use file-check to write the tests.

@csyonghe
Copy link
Collaborator

We need to test for nested functions calls too.

If f calls g which has x byte of context, how is that reported?

@venkataram-nv
Copy link
Collaborator Author

@csyonghe The following is what it looks like for nested calls:

tests/autodiff/reverse-nested-calls.slang(19): note: checkpointing context of 4 bytes was generated for function: 'f'
float f(float x)
      ^
tests/autodiff/reverse-nested-calls.slang(21): note: 4 bytes (float) used to checkpoint the following item:
    return 3.0f * g(2.0f * x);
                   ^

@@ -425,6 +429,9 @@ IRFunc* DiffUnzipPass::extractPrimalFunc(
inst->getFullType(),
intermediateVar,
structKeyDecor->getStructKey());

val->sourceLoc = user->sourceLoc;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of setting source locs everywhere, consider use IRBuilderSourceLocRAII sourceLocInfo(getBuilder(), loc); to simplify the code. This is an RAII object that will make all future insts emited by the builder automatically have the loc as source loc, until the RAII object goes out of scope.

csyonghe
csyonghe previously approved these changes Sep 18, 2024
source/slang/slang-ir-autodiff-transpose.h Outdated Show resolved Hide resolved
source/slang/slang-ir-autodiff-transpose.h Outdated Show resolved Hide resolved
source/slang/slang-ir-autodiff-rev.cpp Outdated Show resolved Hide resolved
saipraveenb25
saipraveenb25 previously approved these changes Sep 18, 2024
@csyonghe csyonghe merged commit b808aa4 into shader-slang:master Sep 19, 2024
12 checks passed
@venkataram-nv venkataram-nv deleted the report-ad-checkpoint-info branch September 19, 2024 14:37
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