Skip to content

Use TORCH_CHECK() instead of throwing std::runtime_error in XLA_CHECK*() macros' implementation.#9542

Merged
ysiraichi merged 10 commits intomasterfrom
ysiraichi/use-torch-check-on-xla-debug-macros
Aug 8, 2025
Merged

Use TORCH_CHECK() instead of throwing std::runtime_error in XLA_CHECK*() macros' implementation.#9542
ysiraichi merged 10 commits intomasterfrom
ysiraichi/use-torch-check-on-xla-debug-macros

Conversation

@ysiraichi
Copy link
Copy Markdown
Collaborator

@ysiraichi ysiraichi commented Aug 5, 2025

This PR replaces the XLA debug macro implementation to use PyTorch's TORCH_CHECK instead of throwing std::runtime_error directly, which makes it so XLA_CHECK*() macros show PyTorch C++ stacktrace.

Key changes:

  • Updated tf_logging.cpp to use TORCH_CHECK instead of std::runtime_error
  • Updated test to match new PyTorch stacktrace format
  • Moved THROW_RUNTIME_ERROR_FROM_C10_ERROR macro to cpp_test_utils.h, and added _ to its name, indicating an implementation internal macro
  • Add deprecation comment to the XLA_CHECK*() macros
    • Use status propagation instead
  • Fixed GetComputationClientOrDie() (at runtime.h) deprecation warning

@ysiraichi ysiraichi requested review from ghpvnist and zhanyong-wan and removed request for ghpvnist August 5, 2025 14:41
@ysiraichi ysiraichi force-pushed the ysiraichi/use-torch-check-on-xla-debug-macros branch from 4864df4 to 389e63c Compare August 5, 2025 14:45
Copy link
Copy Markdown
Collaborator

@ghpvnist ghpvnist left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment thread test/cpp/test_debug_macros.cpp
Comment thread torch_xla/csrc/runtime/debug_macros.h Outdated
Comment thread test/cpp/test_status_common.h Outdated
Copy link
Copy Markdown
Collaborator

@zhanyong-wan zhanyong-wan left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment thread test/cpp/test_status_common.h Outdated
Comment thread test/cpp/test_debug_macros.cpp Outdated
Comment thread test/cpp/test_debug_macros.cpp Outdated
Comment thread test/cpp/cpp_test_util.h Outdated
Comment thread test/cpp/cpp_test_util.h Outdated
Comment thread test/cpp/test_debug_macros.cpp Outdated
@ysiraichi ysiraichi requested a review from zhanyong-wan August 7, 2025 12:20
…CHECK*()` macros' implementation.

Replaces the XLA debug macro implementation to use PyTorch's
`TORCH_CHECK` instead of throwing `std::runtime_error` directly, which
makes it so `XLA_CHECK*()` macros show PyTorch C++ stacktrace.

Key changes:
- Updated tf_logging.cpp to use `TORCH_CHECK` instead of
`std::runtime_error`
- Updated test to match new PyTorch stacktrace format
- Deprecated `ConsumeValue` function in favor of `GetValueOrThrow`
@ysiraichi ysiraichi force-pushed the ysiraichi/use-torch-check-on-xla-debug-macros branch from c14318f to 32475c8 Compare August 7, 2025 14:45
Copy link
Copy Markdown
Collaborator

@zhanyong-wan zhanyong-wan left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment thread test/cpp/test_debug_macros.cpp
Comment thread test/cpp/cpp_test_util.h Outdated
Comment thread test/cpp/test_status_common.h Outdated
@ysiraichi
Copy link
Copy Markdown
Collaborator Author

Here's a quick summary of the last commits:

  • Removed XLA_THROW_RUNTIME_ERROR_FROM_C10_ERROR macro in favor of clearer try-catch tests
  • Renamed testing namespace to cpp_test, following existing tests

@ysiraichi ysiraichi requested a review from zhanyong-wan August 7, 2025 17:27
Copy link
Copy Markdown
Collaborator

@zhanyong-wan zhanyong-wan left a comment

Choose a reason for hiding this comment

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

Thanks!

@ysiraichi ysiraichi merged commit 8c1449f into master Aug 8, 2025
39 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants