Skip to content

Conversation

Maetveis
Copy link
Contributor

@Maetveis Maetveis commented Sep 22, 2025

Starting from #18684 in debug/release with asserts builds entry points are always dumped in ESIMDPostSplitProcessing. This is meant to be a debugging tool, but now it is spamming build logs.
#18684 is marked as NFC, so my assumption is that this was unintentional.
Guard the dump with a variable (disabled by default) so that it can be enabled for debugging, the same way as it was in sycl-post-link.cpp prior to the code move.

FYI: LLVM_ENABLE_DUMP is an option documented to

Enable dump functions even when assertions are disabled

(from llvm/CMakeLists.txt:698)

and its typical usage is to guard the definitions of dump functions, e.g.:

class Foo {
// ...
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
  void dump() const;
#endif
}

and not to guard calls to such functions.

Starting from intel#18684 in debug/release
with asserts builds entry points are always dumped in ESIMDPostSplitProcessing.
This is meant to be a debugging tool, but now it is spamming build
logs.
intel#18684 is marked as NFC, so my
assumption is that this was unintentional.
Guard the dump with a variable (disabled by default) so that it can be
enabled for debugging, the same way as it was in sycl-post-link.cpp prior
to the code move.

FYI: `LLVM_ENABLE_DUMP` is an option documented to
> Enable dump functions even when assertions are disabled

(from [llvm/CMakeLists.txt:698](https://github.com/llvm/llvm-project/blob/32b1f167fbee28debc7527b939a6764575c854a4/llvm/CMakeLists.txt#L698))

and its typical usage is to guard the definitions of dump functions, e.g.:
```cpp
class Foo {
// ...
  void dump() const;
}
```
and not to guard **calls** to such functions.
Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

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

lgtm but we should wait for maksim's review

Copy link
Contributor

@maksimsab maksimsab left a comment

Choose a reason for hiding this comment

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

LGTM.

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