search

malb / m4ri

M4RI is a library for fast arithmetic with dense matrices over GF(2)
GNU General Public License v2.0
49 stars 24 forks source link

CFLAGS in m4ri.pc #9

Open cbouilla opened 2 years ago

cbouilla commented 2 years ago

Reported by @kiwifb 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 CFLAGS 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.

cbouilla commented 2 years ago

@malb said: "some code might be inlined reside in headers, so an application might need the SIMD flags?"

cbouilla commented 2 years ago

My own opinion: compare with the GMP. You don't have to use fancy compile flags, you just have to do -lgmp at link-time. Ideally, M4RI should be the same. We shouldn't export weird flags to the "customer". This means that we shouldn't have customer-facing "static inline" functions defined in headers that do non-trivial things with weird ISA extensions.

malb commented 2 years ago

If we can make that work, that'd be cool.