Bug 34748

I try with 0.20.2 and 0.21.6

The issue is in configure.
When using --disable-openmp or letting openmp test to run, result is the same. 
Even when configure find
checking for gcc option to support OpenMP... unsupported

#define USE_OPENMP 1 is pushed into config.h

I try outside my chroot with ubuntu gcc-4.4.3 that support openmp and using ./configure --disable-openmp
But "#define USE_OPENMP 1" is still pushed to config.h

pixman doesn't have any --disable-omp option, so I guess what happens is that the test program does link correct despite the warnings.

There is no need for --disable-omp, only a --disable-openmp option that work may have some usage, somewhat limited as openmp is only used during pixman tests. Main usage of the openmp configure tests should be to detect if openmp is supported and switch on|off that setting.

Yes pixman tests link and run ok.

I only try to use --disable-openmp because ./configure --help say that exist.

If I modify configure like this before running configure
sed -i 's/define USE_OPENMP 1/undef USE_OPENMP/' configure, there is no more the pragma warnings. So the problem is to fix configure.ac logic.

The real problem is that the configure logic is broken and always push USE_OPENMP define in config.h, even when the compiler does not support openmp (for example gcc compiled with --disable-libgomp).
When test run during configure find
conftest.c:5:17: error: omp.h: No such file or directory
or
gcc: language openmp not recognized
that should not result in "#define USE_OPENMP 1" in config.h
But in reality, this define is always there.

You should be able to test yourself, by just renaming /usr/lib/gcc/$(gcc -dumpmachine)/$(gcc -dumpversion)/include/omp.h
configure will report OpenMP as unsupported, but will not switch off USE_OPENMP. 

The only other program I compile that use openmp is gettext-0.18.1.1.
There, configure correctly find that openmp is not supported and there is no issue. That's in gettext-0.18.1.1/gettext-tools/gnulib-m4/gnulib-comp.m4, only AC_OPENMP macro is called.

I try to figure what to change to configure.ac, but not with much success actually.

configure.ac have
AC_PREREQ([2.57])

Should not that be changed to 2.62 as the first version supporting AC_OPENMP?

Created attachment 44146 [details] [review]
Unrelated fix for missing Werror test result display

Created attachment 44147 [details]
Fix OpenMP not supported case but PIXMAN_LINK_WITH_ENV did not fail

Diff is a bit big only due to the space shift.
There is another way using -Werror -Wall added to PIXMAN_LINK_WITH_ENV

Thanks for the patches - I have a couple of questions.

When you say:
    
    Suppress unneded m4_ifdef([AC_OPENMP], [AC_OPENMP]) and use simply AC_OPENMP

doesn't this require AC_PREREQ(2.62)? Otherwise, AC_OPENMP() may not be defined, right?

Also, you say:

    Not compiled, there is some libtool issue on autoreconf
    autoconf-2.68 trigger too numerous
    warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call detected in body

Do you mean pixman doesn't compile if you use autoconf 2.68?

That the first time I code on configure.ac.

I didn't understand well at that time m4_ifdef([AC_OPENMP], [AC_OPENMP]).
That's true that using simply AC_OPENMP require to change AC_PREREQ([2.62]).

I didn't test the case if AC_OPENMP macro did not exist, but using --disable-openmp should not be very different.

I was wrong with libtool advice to declare a m4 directory. This is not a warning, only an advice and not mandatory to follow. Since the mmx missing ] fix push to the tree, that compile fine with my changes.

I have mostly found the way to fix autoconf-2.68 warnings, adding some AC_LANG_PROGRAM (prologue, body).

I still need to split C code in prologue and body parts, in particular with PIXMAN_LINK_WITH_ENV usage.

This is sometime not easy with some #if define #else that traverse prologue and body like with

#if defined(__GNUC__) && (__GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 7))
/* attribute 'constructor' is supported since gcc 2.7, but some compilers
 * may only pretend to be gcc, so let's try to actually use it
 */
static int x = 1;
static void __attribute__((constructor)) constructor_function () { x = 0; }
int main (void) { return x; }
#else
#error not gcc or gcc version is older than 2.7
#endif

Sorry to be unresponsive for a while.

The _yesno/yesno patch seems correct to me. Would you mind sending it to the pixman mailing list (pixman@lists.freedesktop.org)? If nobody spots any issues with it, I'll merge it.

The other patch has the right idea, I think, but either the m4_ifdef() should not be deleted, or alternatively, AC_PREREQ(2.62) should be added. Can you send that to the list too with this issue fixed?

(In general, we tend to prefer patches to be sent to the mailing list, where it is easy to quote them for review).

Thanks,

ok, I will send tomorrow the patches. I agree for m4_ifdef() needed for openmp.

Not fully ready today to fix all autoconf-2.68 warnings.

Any progress here? It would be useful to get these patches reviewed soon so that they can go into 0.22.0.

I send the 2 patches but messages wait for moderator approval.
I only changed some comments and let unchanged
OPENMP_CFLAGS=
m4_ifdef([AC_OPENMP], [AC_OPENMP])

First line may be needed on autoconf<2.62 and second avoid configure to stop when autoconf<2.62

Cool, thanks. The patches look good to me; I'll push them unless someone complains on the mailing list.

Fixed in 0.21.8.