do not allow implicit conversions between unrelated vector types

[llvmbot]

Bugzilla Link	17164
Version	trunk
OS	All
Depends On	#42682 #42686
Reporter	LLVM Bugzilla Contributor
CC	@Axel-Naumann,@DougGregor,@RKSimon,@riccibruno,@zygoloid,@rotateright

Extended Description

Please see http://stackoverflow.com/questions/9906663/clang-error-ambiguous-conversion-for-static-cast/18693225#18693225

Similar to bug 7934, but still present in clang 3.4

Same error occurs when compiling vector class library with clang:
http://agner.org/optimize/#vectorclass

[zygoloid]

The testcase in the original post on stackoverflow is ill-formed (and g++ and EDG also reject it. It reduces to this:

struct B {                    
  B(const B &);
  B(const unsigned short &);
};

struct A {                         
  operator B() const;
  operator int() const;
};

B process() {                                           
  return static_cast<B>(A());
}

... which is ambiguous, because the conversion using A::operator B() and the conversion using A::operator int() are incomparable: [over.ics.rank] (13.3.3.2)/3 says "User-defined conversion sequence U1 is a better conversion sequence than another user-defined conversion sequence U2 if they contain the same user-defined conversion function and [...]".

Can you provide a testcase from your vector class library?

[llvmbot]

Thank you for a prompt reply. You are right, the example on stackoverflow also fails on g++. Here is the reduced testcase from my vector class library:

#ifdef __GNUC__
    #include <x86intrin.h> 
#else
    #include <immintrin.h>
#endif

class Vec4i  {
protected:
    __m128i xmm;
public:
    Vec4i() {};
    Vec4i(__m128i const & x) {
        xmm = x;
    };
};

class Vec4fb {
protected:
    __m128 xmm;
public:
    Vec4fb() {};
    operator __m128() const { 
        return xmm;
    }
    operator Vec4i() const {
        return _mm_castps_si128(xmm);
    }
};


int main() {
    Vec4fb a;
    Vec4i b = Vec4i(a);
    return 0;
}

/*
clang_problem.cpp:36:15: error: ambiguous conversion for functional-style cast from 'Vec4fb' to 'Vec4i'
    Vec4i b = Vec4i(a);
              ^~~~~~~
clang_problem.cpp:10:7: note: candidate is the implicit copy constructor
class Vec4i  {
      ^
clang_problem.cpp:15:5: note: candidate constructor
    Vec4i(__m128i const & x) {
    ^
1 error generated.
*/

This test case works on Gnu, MS and Intel compilers. I can see the reason why it fails on clang: clang allows implicit conversions between __m128, __m128i and __m128d, the other compilers do not.

My code is a universal vector class library which would be very useful to clang users so I hope you can find a solution. I think compilers should be compatible as far as possible, and I would hate to make a special case in the code for clang

[zygoloid]

Thanks for the investigation!

Yes, these implicit conversions are very troubling, both because they are not gcc-compatible, and also because they silently do something quite surprising:

typedef int m128i attribute((vector_size(16)));
typedef float m128 attribute((vector_size(16)));
m128 x;
m128i y = x;

... bitcasts the float vector to an int vector.

[llvmbot]

Can you turn off the implicit conversions between intrinsic vector types? This would be the most straightforward solution.

[llvmbot]

It works with the option -fno-lax-vector-conversions

This option prevents implicit conversions between __m128, __m128i, __m128d and between __m256, __m256i, __m256d.

I think -flax-vector-conversions should be OFF by default

[llvmbot]

Correction: -fno-lax-vector-conversions does solve this specific problem but causes a lot of other problems because it seems to distinguish between different subtypes of __m128i with different integer sizes. This is incompatible with other compilers.

We need to turn off the implicit conversion between __m128, __m128i, __m128d and between __m256, __m256i, __m256d, but allow implicit conversions between __m128i as two 64-bit integers, or four 32-bit integers, or eight 16-bit integers or sixteen 8-bit integers, signed or unsigned.

[llvmbot]

This problem is still not fixed in version 3.5. Please fix it for the sake of compatibility with Gcc, Intel and MS compilers and to avoid inadvertent bit-casts.

I need to know which version of Clang will fix it so that I can make a preprocessing #if with a work-around. This is important for my vector class library, which already has many users (http://www.agner.org/optimize/#vectorclass)

[llvmbot]

Why is this bug still not fixed in llvm/clang version 3.9.0?
Is there a command line option that solves the problem? (see my comment 2013-09-11)

[llvmbot]

I have changed the version to "Trunk" and "All platforms". The Mac people have done nothing on this bug for three years.

Proposed solution:
Make a command line parameter to switch between two options:

1. Vector register intrinsic types are compatible with all other compilers, i.e. __m128 != __m128i != __m128d, __m256 != __m256i != __m256d, __m512 != __m512i != __m512d.
2. Compatible with code written for earlier versions of Clang, i.e. __m128 = __m128i = __m128d, __m256 = __m256i = __m256d, __m512 = __m512i = __m512d.

Preferably, option 1 should be the default.

Make a preprocessing #define that tells if option 1 is activated. This is necessary for C/C++ code that circumvents the bug with #if or #ifdef directives.

[llvmbot]

Has this bug finally been fixed? The behavior seems to have changed on Clang for Apple/Mac version 6.02.

Can anybody confirm this? Has it been fixed in Clang for other platforms too, and if so - which version?

[zygoloid]

We still perform implicit bitcasts in clang trunk. Perhaps Apple has a local patch to change the default for this flag.

I'm curious about this:

We need to [...] allow implicit conversions between
__m128i as two 64-bit integers, or four 32-bit integers, or eight 16-bit
integers or sixteen 8-bit integers, signed or unsigned.

We can certainly add a third mode to -flax-vector-conversions to disallow only int <-> float conversions, but allowing the above conversions to be performed implicitly also seems pretty broken. Do you believe that large amounts of code are relying on these conversions?

[llvmbot]

At last a sensible reaction after 3½ years! Thank you Richard.

We still perform implicit bitcasts in clang trunk. Perhaps Apple has a local
patch to change the default for this flag.

I'm curious about this:

We need to [...] allow implicit conversions between
__m128i as two 64-bit integers, or four 32-bit integers, or eight 16-bit
integers or sixteen 8-bit integers, signed or unsigned.

We can certainly add a third mode to -flax-vector-conversions to disallow
only int <-> float conversions, but allowing the above conversions to be
performed implicitly also seems pretty broken. Do you believe that large
amounts of code are relying on these conversions?

The Gnu, Intel, and Microsoft compilers are all compatible with each other. They distinguish between only three types of 128 bit vectors:
__m128 = 4 * float
__m128d = 2 * double
__m128i = 16 * int8_t, 8 * int16_t, 4 * int32_t, 2 * int64_t, as well as unsigned versions.
Analogously with __m256i and __m512i.

The C/C++ intrinsic functions that come with these compilers all rely on this. There are now thousands of intrinsic functions so we definitely don't want to fork these.

Compatibility between compilers is very important. Nobody wants to make different versions of their code for different compilers and platforms. The costs of testing and maintaining multiple versions of the code are simply too high. So the answer to your question is YES. All existing code that is not developed specifically for Clang relies on the intrinsic functions where the intrinsic vector types are defined as above. (My own vector class library, which is used by many programmers, works with all four compilers with a preprocessing hack for Clang).

We definitely need a command line switch that makes Clang compatible with the other compilers and a preprocessing macro that can be used for testing if this switch is on.

Of course we also need compatibility between Clang for different platforms. Can somebody please investigate what has happened with Apple Clang with regard to these vector types? I am not a Mac expert.

[llvmbot]

*** Bug llvm/llvm-bugzilla-archive#25557 has been marked as a duplicate of this bug. ***

[llvmbot]

This bug is now more than five years old. Is nothing happening?
What can I do to make somebody take action???

[zygoloid]

As of r371805, clang now supports -flax-vector-conversions={all,integer,none} to control whether you get Clang's old "anything goes" behavior, or the integer-only conversions described in comment#12, or no lax conversions.

For now, the default is unchanged, because clang's intrinsics headers break if lax vector conversions are disabled :( That's the next thing to fix.