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

NGEN_ASM support is incomplete #2061

Closed
nwnk opened this issue Aug 27, 2024 · 5 comments
Closed

NGEN_ASM support is incomplete #2061

nwnk opened this issue Aug 27, 2024 · 5 comments
Labels
sighting Suspicious library behavior. Should be promoted to a bug when confirmed

Comments

@nwnk
Copy link
Contributor

nwnk commented Aug 27, 2024

It looks like it simply never gets built. ngen_asm.hpp is the only place that defines NGEN_ASM, and the only place that includes it looks like:

#ifdef NGEN_ASM
#include "ngen_asm.hpp"
NGEN_REGISTER_DECL(NGEN_NAMESPACE::AsmCodeGenerator, /* nothing */)
#endif

Simply defining the macro is not enough, because unlike the per-gen generators there is no ELFCodeGenerator that derives from AsmCodeGenerator.

@nwnk nwnk added the sighting Suspicious library behavior. Should be promoted to a bug when confirmed label Aug 27, 2024
@densamoilov
Copy link
Contributor

densamoilov commented Aug 27, 2024

+@petercad

@petercad
Copy link
Contributor

@nwnk You're right that ngen_asm.hpp isn't used within oneDNN, and actually, it may be deleted. Like the rest of nGEN it's a header-only library and is only built as needed.

@nwnk
Copy link
Contributor Author

nwnk commented Aug 28, 2024

@petercad

@nwnk You're right that ngen_asm.hpp isn't used within oneDNN, and actually, it may be deleted. Like the rest of nGEN it's a header-only library and is only built as needed.

Are we sure? The nGEN README seems to mention assembly generation as a feature. I would have guessed that it was there for a reason like faster kernel compile times. If that doesn't provide any benefit over CLC then I'm all too happy to remove it, I just want to verify that it's truly unwanted.

@petercad
Copy link
Contributor

@nwnk Yes, nGEN is a library used internally in oneDNN, but not all parts of it are used in oneDNN. I verified that oneDNN builds fine without ngen_asm.hpp.

nwnk added a commit to nwnk/barQAA that referenced this issue Sep 3, 2024
This already never gets built, we only use the OpenCL generator.
densamoilov pushed a commit that referenced this issue Sep 6, 2024
This already never gets built, we only use the OpenCL generator.
theComputeKid pushed a commit to theComputeKid/oneDNN that referenced this issue Sep 16, 2024
This already never gets built, we only use the OpenCL generator.
@vpirogov
Copy link
Member

Fixed by #2078.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sighting Suspicious library behavior. Should be promoted to a bug when confirmed
Projects
None yet
Development

No branches or pull requests

4 participants