WIP: Avoid any use of Vc-detected CXX flags

mentioned in merge request !677 (merged)

marked the checklist item Check this similar to !677 (comment 56357) as completed

changed the description

mentioned in issue docker/ci#9

mentioned in issue #163 (closed)

mentioned in commit 19557065

changed the description

regarding the -fabi-version flag:

Vc sets -fabi-version-compat=0 since https://github.com/VcDevel/Vc/commit/77e0bebe8f1064f073e9e739d346bb2460555260

https://github.com/VcDevel/Vc/commit/5498c009db3eb796802b9fdd8e5773fe4f9f5667 introduced Vc_COMPILE_FLAGS:

* The new variable Vc_COMPILE_FLAGS collects all compiler flags that
should be used when compiling code that includes Vc headers.

https://github.com/VcDevel/Vc/commit/5cdb2b95718e226632a00c3ecea21c168e82e4ec changed -fabi-version from 4 to 0
https://github.com/VcDevel/Vc/commit/b4e047e9130693ac580e446b8db4cedb304c0572 introduced -fabi-version=4 with the comment: "we need GCC ABI version 4 to overload on __m128 vs. __m256"

So this is needed because without it, two declarations

void func(__m128);
void func(__m256);

would have the same mangled name and could not be destinguished by the linker.

The -ffp-contract=fast is from https://github.com/VcDevel/Vc/commit/3bd25129f6a3148c8368c0a443f7cd9a963a6612 with the comment:

use -ffp-contract=fast if the compiler recognizes it to generate FMAs

I suppose that should actually be left to the user and not decided by Vc

And the -Wabi flag also is something that should be left to whoever is compiling.

I do think that it is somewhat OK for Vc to set the -fabi-version flags, as without the correct abi version it is impossible to use Vc. It would be better though to detect the abi version and error out if it is insufficient -- this way the user would still have the option to select the precise abi version if there are other constraints imposed by unrelated libraries.

We currently support gcc 5 and clang 3.8.

In gcc 5 (ubuntu 16.04) we have:

'-fabi-version=N'
     Use version N of the C++ ABI.  The default is version 0.
[...]
'-fabi-compat-version=N'
     On targets that support strong aliases, G++ works around mangling
     changes by creating an alias with the correct mangled name when
     defining a symbol with an incorrect mangled name.  This switch
     specifies which ABI version to use for the alias.

     With '-fabi-version=0' (the default), this defaults to 2.  If
     another ABI version is explicitly selected, this defaults to 0.

So, if we'd leave these options out with gcc 5, -fabi-compat-version would default to 2, promting gcc to generate aliases for that abi-version. That may be a problem, because the aliases for entities overloaded on __m128 and __m256 would clash.

With gcc 6, the default for -fabi-compat-version becomes 8.

With clang, Vc does not seem to detect the abi-version flags. Instead it uses these flags (clang 6 on ubuntu 18.04):

Vc_COMPILE_FLAGS=<-Wno-local-type-template-args;-Wno-unnamed-type-template-args;-ffp-contract=fast>

Same flags detected with clang 3.8.1 on debian stretch.

AFAICT, using local or unnamed types as template arguments is OK since C++11, see https://stackoverflow.com/questions/5751977/local-type-as-template-arguments-in-c. So these options should be fine

Trying this MR out on gcc 5/debian jessie runs the tests successfully. So it seems that leaving the abi-version options off is fine.

I'm guessing for this to become a problem you'd need to actually use symbols containing __m128 or __m256 across translation units, such as when compiling them into a library. The tests kind-off do that, they explicitly instantiate tests for different Vc vectors -- just they only ever do that for one abi, i.e. they nether try to mix Vc::Vector for SSE and AVX abis. I should be able to unit-test that explicitly.

It seems that indeed it is possible to trigger the error the abi-version options are meant to prevent:

Step 9/10 : RUN g++ -std=c++11 -march=native -Wabi *.cc -o test
 ---> Running in 3063a44c6941
/tmp/cc37cpHp.o: In function `test(float __vector(8))':
test-m256.cc:(.text+0x0): multiple definition of `test(float __vector)'
/tmp/ccLx6o0e.o:test-m128.cc:(.text+0x0): first defined here
collect2: error: ld returned 1 exit status
The command '/bin/sh -c g++ -std=c++11 -march=native -Wabi *.cc -o test' returned a non-zero code: 1

That was on epic, which supports avx2. The container was with ubuntu 16:04 and g++ (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609

Docker context:

Well, under this circumstance I think it is not a good idea to skip the Vc_COMPILE_FLAGS. I'm going to close this MR, we can reconsider when we drop support for gcc 5.

closed

WIP: Avoid any use of Vc-detected CXX flags

Merge request reports

Activity