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

ROCr prints to stderr during successful execution which then fails llvm lit testing #170

Open
JonChesterfield opened this issue Nov 1, 2023 · 2 comments
Assignees

Comments

@JonChesterfield
Copy link

JonChesterfield commented Nov 1, 2023

ROCr uses a debug_print() macro to say useful things during execution. There's also a debug_warning which is similar.

#ifdef NDEBUG
#define debug_print(fmt, ...)                                                         
  do {                                                                         
  } while (false)
#else
#define debug_print(fmt, ...)                                                         
  do {                                                                                             
    fprintf(stderr, fmt, ##__VA_ARGS__);                                      
  } while (false)
#endif

This interacts badly with LLVM using lit style tests which look at printed output to determine pass/fail. The net effect is a ROCr which was built with asserts on will fail a lot of tests with messages like KFD does not support xnack mode query.

Please either change the default to silent, such that some macro other than NDEBUG needs to be set to build a chatty version of the library, or provide an environment variable that one can use to disable the printing on a debug build of ROCr.

Thanks!

@dayatsin-amd dayatsin-amd self-assigned this Nov 2, 2023
@dayatsin-amd
Copy link
Collaborator

Tracked internally with ticket: SWDEV-430447

@JonChesterfield
Copy link
Author

Thanks! Environment variable to disable the output would work with llvm (slight hassle as lit is reluctant to forward environment variables, but could be done). Environment variable to enable the output would be preferred here.

It might be worth noting that redirecting from stderr to stdout wouldn't solve the problem with integrating with testing infra which reads the output streams since tests set up in that fashion are very likely to read both of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants