CFLAGS in m4ri.pc

[François Bissey]

I have been giving some thought about the presence of SIMD_CFLAGS and OPENMP_CFLAGS in m4ri.pc. The simd flags are not necessary for a customer of m4ri. By including them you are potentially imposing an optimization (useful or not) on a customer of m4ri. The customer doesn’t need these flags to use m4ri compiled with them. It should be up to consumer whether they want to use it in their own code.

‌

openmp is more subtle. You may want to pass -fopenmp (or equivalent) to the linker to make sure the openmp runtime is linked. But when you put it in clfags you may trigger unwanted use of openmp. Let’s take the example of m4rie. m4rie has its own --enable-openmp switch in configure. If someone has an openmp enabled m4ri but doesn’t want to compile m4rie with openmp (for benchmarking for example), the fact that m4ri puts -fopenmp means openmp code path may still be compiled unless they are guarded by HAVE_OPENMP rather than just #pragmas.

I would be tempted to put OPENMP_CFLAGS in Libs only and not in Cflags.

[François Bissey]

Of course on closer examination, m4rie doesn’t have any openmp code that I can see in spite of the configure flag. Copied from m4ri?

[Martin Albrecht]

Yep, copied from M4RI. Doesn’t it need to be passed through, though? Also, some code might be inlined reside in headers, so an application might need the SIMD flags?

[François Bissey]

Fair comment. M4RI_HAVE_SSE2 is present in the headers that are installed so it is probably a good idea to at least include that corresponding flag. You have made no real customization with the other simd operations though. The compiler is deciding what to do with the libraries by itself with the non sse2 ones.

I am not after immediate changes in any case.

[Martin Albrecht]

Okay, I’m about to make a new release, is there any change you’d propose now? I can see the point about the other flags that aren’t sse2, but does it require a change/update? Just checking to avoid having to do another release soon after.

[François Bissey]

I don’t see this as a pressing issue. And it would potentially require some deeper treatment in any case. From what I can see you used AX_EXT just to get the right sse2 flags and you inherited all the other flags that were available at the time. If you were to update the macro you would potentially enable many more flags. But proper treatment of the issue of just getting sse2 flags means switching off AX_EXT to something else altogether.

That’s not something I want to cook for tomorrow. So let’s leave it for later.

[Martin Albrecht]

Makes sense, thanks.