The macros to convert trans/notrans etc were not correct for use inline

ICLDisco / dplasma

DPLASMA is a highly optimized, accelerator-aware, implementation of a dense linear algebra package for distributed heterogeneous systems. It is designed to deliver sustained performance for distributed systems where each node featuring multiple sockets of multicore processors, and if available, accelerators, using the PaRSEC runtime as a backend.

Other

11 stars 9 forks source link

The macros to convert trans/notrans etc were not correct for use inline #121

Closed abouteiller closed 2 months ago

abouteiller commented 3 months ago

They would cause compilation error when used as functions (e.g. as parameters in a cublass kernel) due to having semicolons etc. Reworded so that they operate as functions (not made them inline because that would cause pulling a bunch of includes when not used.

abouteiller commented 3 months ago

Where are these macros used as functions ? As I commented on the other PR, the compound expressions with values is a GNU extensions, and the correct format is with the ({ ... }).

They will be in the trsm code (you saw that later), and there will also be more of these uses when we switch to cublas_v2 as passing the literal 'N' will not work anymore.

We do not need the GNU extension for macro-functions (we don't have local variables), so I just used the serial composition operator (,) and that is just regular C.

abouteiller commented 2 months ago

Rework:

Joseph preferred these macros to be converted to functions (they did look like functions, but had side effects and couldn't actually be used as functions, confusing)
HIPBLAS defines HIPBLAS_FILL etc to the same values as CBLAS, which is what dplasma does as well, so there is no need for conversion at all for HIP, removed them.

bosilca commented 2 months ago

They did not look like functions, you tried to convert them to look like functions.
In general such usage makes the code less debug-friendly, but in this case the functions are short enough.
I prefer to have the HIP code in there as it makes the code more readable less error prone in the long term (by not requiring users to know that DPLASMA and HIP has in 2024 the same values for the constants) and more extendable in the future. Moreover, a good compiler will optimize out unnecessary code anyway.

QingleiCao commented 2 months ago

What's the problem here? (I remember you told me sometime 😅 @abouteiller )