Skip to content

Main dev load - #1519

Closed
ohblacks wants to merge 1 commit into
triton-lang:main-devfrom
ohblacks:main-dev-load
Closed

Main dev load#1519
ohblacks wants to merge 1 commit into
triton-lang:main-devfrom
ohblacks:main-dev-load

Conversation

@ohblacks

Copy link
Copy Markdown
Contributor

New contributor declaration

  • I am not making a trivial change, such as fixing a typo in a comment.

  • I have written a PR description following these
    rules.

  • I have run pre-commit run --from-ref origin/main --to-ref HEAD.

  • Select one of the following.

    • I have added tests.
      • /test for lit tests
      • /unittest for C++ tests
      • /python/test for end-to-end tests
    • This PR does not need a test because FILL THIS IN.
  • Select one of the following.

    • I have not added any lit tests.
    • The lit tests I have added follow these best practices,
      including the "tests should be minimal" section. (Usually running Python code
      and using the instructions it generates is not minimal.)

@github-actions github-actions Bot added compiler Changes to C/C++ compiler backend (lib/, include/) python Changes to Python runtime or bindings ascend-backend Changes to the Ascend NPU backend labels Aug 12, 2026
Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

🔍 OpenCodeReview found 1 issue(s) in this PR.

  • ✅ Successfully posted inline: 1 comment(s)

Comment on lines +315 to +318
if (isSdpaInferKernel) {
LOG_DEBUG("kernel _sdpa_infer_kernel: skip GM load marking");
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[maintainability · medium]
The log message always says "kernel _sdpa_infer_kernel" regardless of which function name triggered the early return. When the match is for "kernel_sdpa_fwd", the log message becomes misleading and would hinder debugging.

Suggestion:

Suggested change
if (isSdpaInferKernel) {
LOG_DEBUG("kernel _sdpa_infer_kernel: skip GM load marking");
return;
}
if (isSdpaInferKernel) {
LOG_DEBUG("skip GM load marking for sdpa infer kernel");
return;
}

return WalkResult::advance();
});
if (isSdpaInferKernel) {
LOG_DEBUG("kernel _sdpa_infer_kernel: skip GM load marking");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[bug · low]
The log message only mentions _sdpa_infer_kernel but the condition also matches kernel_sdpa_fwd. If the module contains kernel_sdpa_fwd (but not _sdpa_infer_kernel), this log message would be misleading—it would print "kernel _sdpa_infer_kernel: skip GM load marking" even though the actual matching function is kernel_sdpa_fwd. Consider making the log message generic or dynamically including the matched function name.

Suggestion:

Suggested change
LOG_DEBUG("kernel _sdpa_infer_kernel: skip GM load marking");
LOG_DEBUG("skip GM load marking for sdpa kernel");

@ohblacks ohblacks closed this Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ascend-backend Changes to the Ascend NPU backend compiler Changes to C/C++ compiler backend (lib/, include/) python Changes to Python runtime or bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant