Open MPI logo

Hardware Locality Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Hardware Locality Development mailing list

Subject: Re: [hwloc-devel] 1.3.2rc1 has escaped
From: Paul H. Hargrove (PHHargrove_at_[hidden])
Date: 2012-02-09 14:27:59


Jeff,

What you have for the "Make sure..." is wrong in the same way as the one
that was in rc1.
The problem is that the AC_COMPILE_IFELSE code tests too-few and
too-many args together.
Since xlc makes too many an error by default, we don't notice its
MISbehavior when given too few.
So, one needs to split the too-many and too-few tests as I did in the
patch I sent.

I don't think we should drop that AC_COMPILE_IFELSE entirely (or rather
we shouldn't drop the TWO once split).
If we were to encounter another Linux compiler that didn't STOP on
too-few arguments the binding code would get silently broken again.

I was also partial to the "structure" of my patch which needed to test
$hwloc_c_vendor only once.
This would allow adding compiler-specific logic in exactly one place if
other cases arise.

I *do* like the way you've run the AC_COMPILE_IFELSE test AFTER adding
the compiler-specific flag (thus confirming that it actually resolved
the problem). However, as noted above you will need to split the
too-few and too-many arg tests for that to be effective.

And regarding the "older, buggy" comment:
This is a recent XLC compiler, and this behavior is NOT a bug because
the C spec doesn't require a fatal error here.
That is why I commented (with <rant> delimiters) on the evils of
configure probes that try to determine how many arguments appear in a
prototype.

-Paul

On 2/9/2012 5:08 AM, Jeff Squyres wrote:
> How's this patch (against v1.3, assuming
> https://svn.open-mpi.org/trac/hwloc/changeset/4285)?
>
> Is the test that checks to see if compilers error when the wrong number of params are passed now mooot?
>
> Index: config/hwloc.m4
> ===================================================================
> --- config/hwloc.m4 (revision 4285)
> +++ config/hwloc.m4 (working copy)
> @@ -268,22 +268,24 @@
> AS_IF([test "$HWLOC_VISIBILITY_CFLAGS" != ""],
> [AC_MSG_WARN(["$HWLOC_VISIBILITY_CFLAGS" has been added to the hwloc CFLAGS])])
>
> - # make sure the compiler returns an error code when function arg count is wrong,
> - # otherwise sched_setaffinity checks may fail
> + # Make sure the compiler returns an error code when function arg
> + # count is wrong, otherwise sched_setaffinity checks may fail.
> + # For older, buggy versions of the xlc compilers, we need to set
> + # an additional compiler flag to catch these situations.
> + AS_IF([test "$hwloc_c_vendor" = "ibm"],
> + [HWLOC_CFLAGS_save=$CFLAGS
> + CFLAGS="$CFLAGS -qhalt=e"])
> AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
> extern int one_arg(int x);
> extern int two_arg(int x, int y);
> int foo(void) { return one_arg(1, 2) + two_arg(3); }
> ]])], [
> AC_MSG_WARN([Your C compiler does not consider incorrect argument counts to be a fatal error.])
> - if test "$hwloc_check_compiler_vendor_result" = "ibm"; then
> - AC_MSG_WARN([For XLC you may try appending '-qhalt=-e' to the value of CFLAGS.])
> - AC_MSG_WARN([Alternatively you may configure with a different compiler.])
> - else
> - AC_MSG_WARN([Please report this failure, and configure using a different C compiler if possible.])
> - fi
> AC_MSG_ERROR([Cannot continue.])
> ])
> + # Restore the CFLAGS if we modified them above
> + AS_IF([test "$hwloc_c_vendor" = "ibm"],
> + [CFLAGS=HWLOC_CFLAGS])
>
> #
> # Now detect support
> @@ -387,6 +389,12 @@
> AC_DEFINE_UNQUOTED(hwloc_thread_t, $hwloc_thread_t, [Define this to the thread ID type])
> fi
>
> + # For older, buggy versions of the xlc compilers, we need to set
> + # an additional compiler flag to catch cases where the wrong
> + # number of parameters are passed.
> + AS_IF([test "$hwloc_c_vendor" = "ibm"],
> + [HWLOC_CFLAGS_save=$CFLAGS
> + CFLAGS="$CFLAGS -qhalt=e"])
> _HWLOC_CHECK_DECL([sched_setaffinity], [
> AC_DEFINE([HWLOC_HAVE_SCHED_SETAFFINITY], [1], [Define to 1 if glibc provides a prototype of sched_setaffinity()])
> AC_MSG_CHECKING([for old prototype of sched_setaffinity])
> @@ -403,6 +411,9 @@
> #define _GNU_SOURCE
> #include<sched.h>
> ]])
> + # Restore the CFLAGS if we modified them above
> + AS_IF([test "$hwloc_c_vendor" = "ibm"],
> + [CFLAGS=HWLOC_CFLAGS])
>
> AC_MSG_CHECKING([for working CPU_SET])
> AC_LINK_IFELSE([
>
>
>
>
> On Feb 8, 2012, at 7:47 PM, Paul H. Hargrove wrote:
>
>>
>> On 2/8/2012 4:41 PM, Paul H. Hargrove wrote:
>>> I do agree w/ Samuel that the BEST solution is to apply "-qhalt=e" ONLY to the test(s) where one expects the compiler to through errors (rather than warnings) for function calls with argument counts which don't match the prototypes. At the moment, I am 90% certain that the "old sched_setaffinity()" probe is the only one fitting that description.
>> I am hoping to be able contribute patch for this soon.
>> -Paul
>>
>> --
>> Paul H. Hargrove PHHargrove_at_[hidden]
>> Future Technologies Group
>> HPC Research Department Tel: +1-510-495-2352
>> Lawrence Berkeley National Laboratory Fax: +1-510-486-6900
>>
>> _______________________________________________
>> hwloc-devel mailing list
>> hwloc-devel_at_[hidden]
>> http://www.open-mpi.org/mailman/listinfo.cgi/hwloc-devel
>

-- 
Paul H. Hargrove                          PHHargrove_at_[hidden]
Future Technologies Group
HPC Research Department                   Tel: +1-510-495-2352
Lawrence Berkeley National Laboratory     Fax: +1-510-486-6900