search

microsoft / knossos-ksc

Compiler with automatic differentiation
Other
45 stars 10 forks source link

Annotate each C++ function with a macro #1014

Closed dcrc2 closed 2 years ago

dcrc2 commented 2 years ago

As explained in this comment at the top of knossos.h:

Each C++ function is annotated with one of the following macros:
- KS_DEF: a definition which has been generated by ksc
- KS_FUNCTION: a function used only from within ksc-generated code
- KS_INLINE_FUNCTION: an inline KS_FUNCTION
- KS_INTERFACE: a function which may be called from outside the generated code (e.g. to convert to/from ks types)

The aim is to allow the generated C++ files to be used as CUDA files without modifying ksc. This can be achieved by #define KS_DEF __device__, and similarly for the other macros. The CUDA support is not added by this PR: I've just added the macros on their own to confirm that there are no breaking changes.

awf commented 2 years ago

I find KS_INLINE_FUNCTION a bit odd. Do the others not have INLINE versions because we know they will never be needed, or because they are not currently needed?

Perhaps the opening comment could also show the CUDA expansion for each macro?

awf commented 2 years ago

Would a separate KS_INLINE work?

awf commented 2 years ago

This can be achieved by #define KS_DEF device, and similarly for the other macros.

Maybe add some of this discussion to the opening comment

dcrc2 commented 2 years ago

I find KS_INLINE_FUNCTION a bit odd. Do the others not have INLINE versions because we know they will never be needed, or because they are not currently needed?

Not currently needed - the KS_INTERFACE functions are all either templates or class member functions.

I can replace with inline KS_FUNCTION if that looks better. My only worry was having to decide between inline KS_FUNCTION and KS_FUNCTION inline: in fact both of these work for the CUDA support, but I wasn't sure whether if we wanted to play around with adding other attributes, they might be more picky about whether they appear before or after the inline keyword.

dcrc2 commented 2 years ago

I can replace with inline KS_FUNCTION if that looks better.

I've pushed a change to do it this way.

dcrc2 commented 2 years ago

This can be achieved by #define KS_DEF __device__, and similarly for the other macros.

Maybe add some of this discussion to the opening comment

OK I've pulled in another commit from #976, which defines these macros for CUDA and non-CUDA code. These definitions come immediately after the comment so hopefully this makes the purpose more clear.