sw: Adjust `*_decls` headers for usability in downstream projects

[zero9178]

Downstream projects wishing to use the snitch runtime via its *_decls headers have so far run into a few issues:

• Not all headers correctly include the C standard headers of entities they depend on
• There were no header guards or similar present, making multiple includes an error in many cases

This PR fixes these issues by:

• #includeing required headers
• Adding #pragma once to all headers

As a further drive-by, the riscv_decls.h header was fixed to be usable without linking errors the same way as other files

[colluca]

Thanks for the valuable contribution @zero9178!

I absolutely agree with you on every point. My only suggestion would be, rather than to remove the inline specifiers, to define the missing functions in the relevant translation units (most .h files already have a corresponding .c file, but otherwise one can be created).

Would you be able to implement this? Also, it seems the software build fails in the CI and there are some linting errors. I can take care of the latter.

[zero9178]

Thanks for the valuable contribution @zero9178!

I absolutely agree with you on every point. My only suggestion would be, rather than to remove the inline specifiers, to define the missing functions in the relevant translation units (most .h files already have a corresponding .c file, but otherwise one can be created).

Thank you so much for taking a look! I am curious, as this review comment raises a question for me: Is a downstream project (say a user writing a library with a packaged snitch-toolchain) supposed to include the *_decls.h headers or the headers in the src directory? Furthermore, what would be the advantage of keeping the inline specifier on the declarations? Besides it not being allowed if no definition is visible (something true for every C file including e.g. alloc_decls.h but not alloc.h), I was under the impression that it does not have any semantics if not on a definition. Edit: Turns out not having it on a declaration causes the multiple definitions error as the compiler will generate the inline body as soon as it finds a declaration that does not have inline on it.

Also, it seems the software build fails in the CI and there are some linting errors. I can take care of the latter.

No need, thank you! In the process I also added #pragma once to the headers in the src directory. CI should be fixed now as well :crossed_fingers: (big fan of these CI errors btw)

[zero9178]

Okay it seems like just the LTO tests are failing now. I think for the meantime your suggestion of keeping it probably makes sense for the purpose of landing all the other changes and it can always be revisited in the future.

[colluca]

Thank you so much for taking a look! I am curious, as this review comment raises a question for me: Is a downstream project (say a user writing a library with a packaged snitch-toolchain) supposed to include the *_decls.h headers or the headers in the src directory?

The idea is that downstream projects can build their own header-only library based on these sources or use the compiled library. The sources in sw/snRuntime should be general enough to support multiple Snitch-based targets. And dedicated implementations can be created from these base sources (see the Banshee runtime and the RTL runtime as example implementations). They might need both the *_decls.h and src/*.h headers. The purpose of the first is to provide forward declarations, since the fact that the definitions appear in the src/*.h files (for inlining) complicates the design (difficult to break circular dependencies).

Furthermore, what would be the advantage of keeping the inline specifier on the declarations? Besides it not being allowed if no definition is visible (something true for every C file including e.g. alloc_decls.h but not alloc.h), I was under the impression that it does not have any semantics if not on a definition.

Oh, if you removed it only on declarations then you're right, and you can ignore my comment :)

[zero9178]

Thank you so much for taking a look! I am curious, as this review comment raises a question for me: Is a downstream project (say a user writing a library with a packaged snitch-toolchain) supposed to include the *_decls.h headers or the headers in the src directory?

The idea is that downstream projects can build their own header-only library based on these sources or use the compiled library. The sources in sw/snRuntime should be general enough to support multiple Snitch-based targets. And dedicated implementations can be created from these base sources (see the Banshee runtime and the RTL runtime as example implementations). They might need both the *_decls.h and src/*.h headers. The purpose of the first is to provide forward declarations, since the fact that the definitions appear in the src/*.h files (for inlining) complicates the design (difficult to break circular dependencies).

Thanks for the elaboration, that really helps!

Furthermore, what would be the advantage of keeping the inline specifier on the declarations? Besides it not being allowed if no definition is visible (something true for every C file including e.g. alloc_decls.h but not alloc.h), I was under the impression that it does not have any semantics if not on a definition.

Oh, if you removed it only on declarations then you're right, and you can ignore my comment :)

This is what I originally did but it turns out that it is actually required to not cause multiple definitions. Clang is fine with it for now. I've adjusted the PR description to reflect the smaller scope of the PR. Thank you for your patience and sorry for the noise