bugGNU Octave - Bugs: bug #44346, configure summary shows OpenMP...

 
 

bug #44346: configure summary shows OpenMP enabled even if not detected

Submitted by:  Mike Miller <mtmiller>
Submitted on:  Sun 22 Feb 2015 11:26:00 PM UTC  
 
Category:  Configuration and Build System Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Documentation
Status:  Fixed Assigned to:  Mike Miller <mtmiller>
Originator Name:  Open/Closed:  Closed
Release:  dev Operating System:  Any

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Mon 23 Feb 2015 02:20:07 PM UTC, comment #10: 

The problem with adding an argument to AX_OPENMP is that the HAVE_OPENMP config.h symbol is no longer defined. That's something we had in the OCTAVE_CHECK_OPENMP macro and is also conveniently provided by AX_OPENMP, but only if you don't give it a first argument.

Mike Miller <mtmiller>
Project AdministratorIn charge of this item.
Mon 23 Feb 2015 02:16:06 PM UTC, comment #9: 

Yes, we should ultimately remove the XTRA_ flags.

Mike's comments about the reason for leaving CFLAGS untouched are what I was about to write.

However, it may be true that subsequent tests would pass or fail with or without the use of things like the openmp or pthreads flags.  See the changeset I just pushed.

http://hg.savannah.gnu.org/hgweb/octave/rev/d0c7b5346d55

John W. Eaton <jwe>
Project Administrator
Mon 23 Feb 2015 02:08:46 PM UTC, comment #8: 

Nevermind, I see that configure's feature test macros do not use AM_CFLAGS, AM_CXXFLAGS, etc. So any compiler options that influence the pass/fail of tests further on in the script need to be in CFLAGS / CXXFLAGS. Might the presence or absence of -fopenmp influence some later library tests? I don't know.

Mike Miller <mtmiller>
Project AdministratorIn charge of this item.
Mon 23 Feb 2015 02:05:42 PM UTC, comment #7: 

Should we move this discussion somewhere else?

I think John's point was that appending to the user's flags in configure is not a problem, but the user can then run "make CFLAGS='whatever' and the flags that were appended could get lost. The plain CFLAGS and CXXFLAGS are theoretically supposed to be untouched so the user can set them to whatever they want, and the project puts necessary flags in the AM versions.

I think it would be interesting to see if we could move all flags to AM_*FLAGS, including the pthreads flag, the warning flags, OpenMP, etc. John mentioned XTRA_* were used for legacy reasons, could we consolidate those uses also and eliminate those xtra variables?

Would the fact that the pthreads flag is needed early not still be valid if it were in AM_CFLAGS? I think the "early" just means the line number in configure.ac.

And as for Fortran, I think it would be useful, for users to potentially build OpenMP extensions to Octave in Fortran if they like. I already tried that, though, and the test fails with gfortran for F77, so presumably it only works in Fortran 90 or newer standards. So that's not possible unless we use FC instead of F77 (there was a discussion about that some time ago).

Mike Miller <mtmiller>
Project AdministratorIn charge of this item.
Mon 23 Feb 2015 01:49:18 PM UTC, comment #6: 

I don't think it's a problem to append to the user's CFLAGS/CXXFLAGS.  Because the operation is append, the user gets to decide what the base flags are at all times either through an environment variable or passing it into the make invocation.

The pthreads case was special as noted in the comments in configure.ac.

AX_PTHREAD
## Include pthread libs and flags early in case other tests need them.
## They seem to be required for the OpenGL tests on Debian systems.
LIBS="$PTHREAD_LIBS $LIBS"
CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
CXXFLAGS="$CXXFLAGS $PTHREAD_CFLAGS"

So configure itself and the conftest.c files that it produces when testing for various compiler features needed the flag so sticking it in AM_CFLAGS would not have been enough.

I don't think OpenMP has those kinds of affects so you could put it in AM_CFLAGS or keep the current situation.

As for Fortran, most of those libraries were written decades ago and I don't see people going back to re-engineer support for OpenMP.  So I think it would be pretty useless to enable openmp for those files.

Rik <rik5>
Project Administrator
Mon 23 Feb 2015 01:08:23 PM UTC, comment #5: 

One other thing...

Should we be doing the same for Fortran?

John W. Eaton <jwe>
Project Administrator
Mon 23 Feb 2015 01:07:38 PM UTC, comment #4: 

The generated rules are

LINK = $(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) \
        $(LIBTOOLFLAGS) --mode=link $(CCLD) $(AM_CFLAGS) $(CFLAGS) \
        $(AM_LDFLAGS) $(LDFLAGS) -o $@

CXXLINK = $(LIBTOOL) $(AM_V_lt) --tag=CXX $(AM_LIBTOOLFLAGS) \
        $(LIBTOOLFLAGS) --mode=link $(CXXLD) $(AM_CXXFLAGS) \
        $(CXXFLAGS) $(AM_LDFLAGS) $(LDFLAGS) -o $@

So AM_LDFLAGS and LDFLAGS are shared, but the CFLAGS and CXXFLAGS are separate and they are passed in the link step.

So I guess it is sufficient to put these flags in AM_CFLAGS and AM_CXXFLAGS and not worry about LDFLAGS.

I'll do that.

John W. Eaton <jwe>
Project Administrator
Mon 23 Feb 2015 05:30:46 AM UTC, comment #3: 

I followed the model that we use currently for pthreads. So the same problem applies there, configure will detect pthreads, but if someone runs 'make CXXFLAGS="-g -O2"', the pthreads option will be lost and something may not work right. I don't think there's much risk of a problem there.

I suppose it's only possible for the flags to be different if someone uses a different compiler for C and C++ for some reason.

Doesn't Automake do the right thing with CCLD getting CFLAGS and CXXLD getting CXXFLAGS? I haven't checked but that's how I thought it worked.

Mike Miller <mtmiller>
Project AdministratorIn charge of this item.
Mon 23 Feb 2015 04:58:51 AM UTC, comment #2: 

I don't think we should add these flags directly to CFLAGS and CXXFLAGS as those are supposed to default to "-g -O2" and be available for the user to change.  Now if someone runs Make with CFLAGS=-g, won't they also remove the openmp flags from CFLAGS?  That's probably not what they intend.  So they probably should be added to AM_CFLAGS and AM_CXXFLAGS, which for historical reasons are still defined from XTRA_CFLAGS and XTRA_CXXFLAGS.

Also, is it possible that the openmp flags would be different for the C and C++ compilers?  If so, which should be passed to the linker?  I guess for us it would normally be the CXXFLAGS version since we link most programs and libraries with the C++ compiler.

John W. Eaton <jwe>
Project Administrator
Sun 22 Feb 2015 11:51:22 PM UTC, comment #1: 
Rik <rik5>
Project Administrator
Sun 22 Feb 2015 11:26:00 PM UTC, original submission:  

The summary at the end of configure shows "yes" for OpenMP if the user chose to enable it (or enabled by default) but not whether OpenMP was detected with the compiler. This should be a trivial fix to set the variable to "no" if the compiler tests fail to detect OpenMP.

Mike Miller <mtmiller>
Project AdministratorIn charge of this item.

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mtmiller (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-02-22 rik5 StatusNone => Fixed
        Open/ClosedOpen => Closed

    Back to the top


    Powered by Savane 3.5