-
Notifications
You must be signed in to change notification settings - Fork 808
[SYCL][ESIMD] Decorate dump methods according to LLVM guideline. #20269
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
base: sycl
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see title is saying Decorate dump methods according to LLVM guideline
, but in the code I see that in addition to that, it seems dumping of entry points was removed from ESIMD Post split processing. Is it by intention? Could you please provide more details in PR description maybe?
Some context: this change fixes urgent issue with coverage build which is being built with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a question.
Linked.rebuildEntryPoints(Names); | ||
Result.clear(); | ||
Result.emplace_back(std::move(Linked)); | ||
DUMP_ENTRY_POINTS(Result.back().entries(), Result.back().Name.c_str(), 4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just double checking that we don't need this anymore, not even under the appropriate ifdefs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use LLVM_DEBUG(...)
for that. I just don't know whether anyone really needs that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with removing it, just wanted to make sure it was intended.
@intel/llvm-gatekeepers please consider merging |
LLVM uses
if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
for decoration of dumping methods. Dumping is used for debugging but not for testing. Therefore, its usage is removed.