configure should warn or prohibit mixed-family/mixed-version compilers

Issue #518 resolved
Dan Bonachea created an issue

UPC++ configure currently silently accepts mixed-family/mixed-version compiler setups, where CC and CXX specify a different compiler family, or a different major version of the same compiler family.

Example:

dirac$ configure CC=gcc CXX=clang++ --without-mpicc && make -j16 && make -j16 check
Fetching https://gasnet-bugs.lbl.gov/nightly/unlisted/GASNet-stable.tar.gz
Unpacking GASNet-stable.tar.gz

System: Linux pcp-d-5 3.10.0-693.1.1.el7.x86_64 #1 SMP Tue Aug 15 08:36:44 CDT 2017 x86_64 x86_64 x86_64 GNU/Linux
LSB Version:    :core-4.1-amd64:core-4.1-noarch
Distributor ID: Scientific
Description:    Scientific Linux release 7.3 (Nitrogen)
Release:        7.3
Codename:       Nitrogen

Date: Sat Jan 15 15:55:46 PST 2022
Current directory:<redacted>
Install directory: /usr/local/upcxx
Configure command: ../upcxx/configure CC=gcc CXX=clang++ --without-mpicc
Configure environment:
    GASNET='https://gasnet-bugs.lbl.gov/nightly/unlisted/GASNet-stable.tar.gz'
    GASNET_CONFIGURE_ARGS='--disable-kind-cuda-uva'
    GMAKE='/usr/local/pkg/gmake/newest/bin/make'

/usr/local/pkg/clang/12.0.0/bin/clang++
clang version 12.0.0
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/local/pkg/clang/12.0.0/bin
/usr/local/pkg/gcc/11.2.0/bin/gcc
gcc (GCC) 11.2.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

 ...

These "mixed" configures are a Very Bad Idea for several reasons:

  1. We don't test them at all
  2. Many such combinations will break in subtle or confusing ways (including the combo shown above)
  3. All C++ compilers we support come bundled with a matching C compiler, so there's no good reason to not use matched compilers.

Based on a quick skim of INSTALL.md, we don't appear to even mention this important potential pitfall (although we talk plenty about the C++ compiler in isolation). However thanks to our defaulting CC=gcc, this mistake is very easy to accidentally make -- i.e. configure CXX=clang++ is a mixed-family config equivalent to the one shown above!

GASNet configure DOES generate a warning for mixing, ie:

The compiler families of $CC and $CXX do not match:
      CC:  COMPILER_FAMILY:GNU          COMPILER_VERSION:11.2.0
     CXX:  COMPILER_FAMILY:CLANG        COMPILER_VERSION:12.0.0 
If this is not what you intended, then set $CXX to select a different C++ compiler.

but this happens after the UPC++ configure step finishes and it's very easy for this to get lost in the middle of the huge library build step.

I propose we apply some subset of the following corrections in rough order of increasing effort, to help users avoid this problem:

  1. Update INSTALL.md to clarify that mixing compiler families or major versions is NOT supported and highly discouraged
  2. Add configure-time checks for mixed-family/mixed-version compiler setups and issue a loud warning before the end of UPC++ configure.
  3. Better yet default to treating a mixed configure as a fatal error and require an override option for "yes, I really want to do this unsupported thing"
  4. Add a mechanism to grep the GASNet configure warnings and summary from bld/gasnet.debug/config.txt and echo it to the console at the end of libupcxx build. This is probably worth doing for other reasons as well.

Comments (5)

  1. Paul Hargrove

    I agree we can (and so should if time allows) do better for this type of mismatch.

    My initial thoughts and opinions:

    1. This seems like something we should consider a "blocker" for the next release. There is no reason NOT to fix the docs.

    2. This actually sounds to me like the most difficult of the four items. However, I am pretty sure use of portable_platform.h should be sufficient to "fingerprint" the two compilers for comparison purposes.

    3. I think this is just an incremental improvement on item 2. So assuming we agree this is the right approach, it should be simple to add.

    4. Don't know why this was listed last by "rough order of increasing effort".
      The following (to go in the all-hook in bld/Makefile.rules) may need some "cosmetics", but extracts the requested portion of the outputs from both configure runs.

    for x in debug opt; do
      if [[ -d bld/gasnet.$x ]]; then
        echo -n "GASNet $x "
        perl -n -e '$p ||=  m/configure warning summary/;' \
                -e '$p &&= !m/^--------------/;' \
                -e 'print if $p;' \
             -- bld/gasnet.$x/config.txt
      fi
    done
    
  2. Dan Bonachea reporter

    extracts the requested portion of the outputs from both configure runs.

    I was thinking along similar lines, but would additionally include the subsequent "GASNet configuration" summary, since it provides generally useful information. The slightly more "difficult" part would be removing unnecessary duplication in the warnings from gasnet.debug and gasnet.opt (which are often but not always identical), and from the configuration summary (which is basically always the same between debug/opt, so we should only show it once).

  3. Paul Hargrove
    • changed status to open

    Proposed resolution (including items 1, 2 and 3) appears in pull request 396.

    I would like to see item 4 (for printing GASNet's configure summary) split off into a distinct issue.

  4. Paul Hargrove

    Configure time version checks

    This commit resolves issue 518 by ensuring that "mismatched" C and C++ compilers are rejected by default. The INSTALL.md section regarding the configure step is updated to mention the requirement and the (unsupported) means to bypass it.

    → <<cset 369ff491df9a>>

  5. Log in to comment