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

Add prefixes to macros in in config.hpp.in #554

Closed
al42and opened this issue Aug 9, 2024 · 3 comments
Closed

Add prefixes to macros in in config.hpp.in #554

al42and opened this issue Aug 9, 2024 · 3 comments
Assignees
Labels
help wanted Tasks, issues or features that could be implemented and contributed to the project RFC A proposal to add new API

Comments

@al42and
Copy link
Contributor

al42and commented Aug 9, 2024

Summary

To prevent potential macro naming conflicts between oneMKL and client applications, this RFC proposes adding a unique prefix (such as ONEMKL_ or ONEAPI_ONEMKL_) to all macros defined in oneMKL's public API.

Problem statement

oneMKL defines a bunch of configuration macros in config.hpp, which is transitively included in the <oneapi/mkl.hpp> file.

Those are ENABLE_<backend>_BACKEND for each enabled backend, BUILD_SHARED_LIBS and REF_(BLAS|CBLAS)_LIBNAME.

As a consequence, the client application that tries to use oneMKL will also have them all defined. This could be useful, but, since macros don't obey any scope rules, they can also easily cause confusion if the client application itself uses, e.g., ENABLE_CUFFT_BACKEND or BUILD_SHARED_LIBS.

ES.33 in C++ Core Guidelines recommends adding supposedly unique prefixes (e.g., your organization’s name) to the macro names.

As such, I suggest adding ONEMKL_ or ONEAPI_ONEMKL_ prefix to all the macros defined in the public API.

Details

  • Those macros are not part of the specification as far as I can tell, but they can (intentionally or not) affect user applications.
@al42and al42and added the RFC A proposal to add new API label Aug 9, 2024
@Rbiessy
Copy link
Contributor

Rbiessy commented Aug 19, 2024

Thanks for the RFC, I agree this would be nicer. I would suggest using ONEAPI_ONEMKL_ for now. Is this blocking you? We may have the bandwidth to work on this but contributions are welcome if anyone needs this soon.

@al42and
Copy link
Contributor Author

al42and commented Aug 19, 2024

Is this blocking you?

Not blocking, just wanted to raise a concern since it's part of public API (I don't have a link at hand, but it was suggested in one of the examples to check these variables).

We may have the bandwidth to work on this but contributions are welcome if anyone needs this soon.

Ok! 😉

@mkrainiuk mkrainiuk added the help wanted Tasks, issues or features that could be implemented and contributed to the project label Sep 4, 2024
@s-Nick s-Nick mentioned this issue Sep 17, 2024
2 tasks
@s-Nick
Copy link
Contributor

s-Nick commented Oct 15, 2024

#571 addressed this issue

@s-Nick s-Nick closed this as completed Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Tasks, issues or features that could be implemented and contributed to the project RFC A proposal to add new API
Projects
None yet
Development

No branches or pull requests

4 participants