Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Clang discards attributes aligned and may_alias for typedefs passed as template arguments #46643

Open Quuxplusone opened 4 years ago

Quuxplusone commented 4 years ago
Bugzilla Link PR47674
Status NEW
Importance P normal
Reported by Mateusz Zych (mte.zych@gmail.com)
Reported on 2020-09-28 17:37:06 -0700
Last modified on 2020-09-29 10:41:02 -0700
Version 10.0
Hardware All Linux
CC blitzrakete@gmail.com, dgregor@apple.com, erik.pilkington@gmail.com, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97222, PR40810
Hello!

Clang discards aligned attribute, applied on a typdef,
when it's passed as a template argument.

Compiler Expolorer:
 Clang -> https://godbolt.org/z/x375GT

C++ Source Code:
 #include <iostream>

 typedef float vec __attribute__((vector_size(8)));
 typedef float fp  __attribute__((aligned(16)));

 template <typename t> struct identity { typedef t type; };

 int main ()
 {
     std::cout << sizeof(typename identity<vec>::type) << std::endl;
     std::cout << sizeof(vec                         ) << std::endl;

     std::cout << alignof(typename identity<fp>::type) << std::endl;
     std::cout << alignof(fp                         ) << std::endl;
 }

Program Output:
 8
 8
 4
 16

The above program shows that alignment of the fp typedef changes,
after it's been passed through the identity meta-function - it's 4-bytes
instead of expected 16-bytes.

What's interesting is not all type attributes are discarded - the vector_size
attribute is preserved after being passed through the identity meta-function.

This behavior is required, since removal of the vector_size attribute
would be a semantic change of the vec type, affecting even its size,
because the vec type would represent a single float,
instead of a vector of 2 floats.

The same could be said about the aligned attribute - discarding it
is also a semantic change, since alignment is a fundamental property of a type,
affecting among others code generation,
that is, two types are not equivalent if they have different alignment.

This is the reason why I argue that, passing a typedef as a template argument
should preserve its aligned attribute, instead of discarding it.

Moreover, the Intel C++ compiler implements this behavior correctly.

Compiler Expolorer:
 ICC -> https://godbolt.org/z/9vr9se

Program Output:
 8
 8
 16
 16

The issue described above doesn't apply only to the aligned type attribute,
but also to the may_alias type attribute.

Compiler Expolorer:
 Clang -> https://godbolt.org/z/zYEz78

C++ Source Code:
 #include <iostream>
 #include <limits>

 typedef float fp __attribute__((may_alias));

 template <typename T> struct identity { typedef T type; };

 static_assert( sizeof(float) ==  sizeof(int)       , "");
 static_assert(alignof(float) == alignof(int)       , "");
 static_assert(std::numeric_limits<float>::is_iec559, "");

 bool can_alias_float ()
 {
     const auto fn = [] (float *f, int *i) -> int
     {
         *i = 0x1;
         *f = 2.0f; // In ieee754 bin repr of 2.0f is 0x40000000.
         return *i;
     };
     int val; // Casting int* to float* is UB!
     val = fn(reinterpret_cast<float*>(&val), &val);
     return val == 0x40000000;
 }

 bool can_alias_fp ()
 {
     const auto fn = [] (fp *f, int *i) -> int
     {
         *i = 0x1;
         *f = 2.0f; // In ieee754 bin repr of 2.0f is 0x40000000.
         return *i;
     };
     int val; // Casting int* to fp* is OK, due to attribute may_alias.
     val = fn(reinterpret_cast<fp*>(&val), &val);
     return val == 0x40000000;
 }

 bool can_alias_identity_type_fp ()
 {
     const auto fn = [] (typename identity<fp>::type *f, int *i) -> int
     {
         *i = 0x1;
         *f = 2.0f; // In ieee754 bin repr of 2.0f is 0x40000000.
         return *i;
     };       // Casting int* to fp* should be OK,
     int val; // but the attribute may_alias is discarded, causing UB!
     val = fn(reinterpret_cast<typename identity<fp>::type*>(&val), &val);
     return val == 0x40000000;
 }

 int main ()
 {
     std::cout << "fp                 " << can_alias_fp              () << '\n';
     std::cout << "identity<fp>::type " << can_alias_identity_type_fp() << '\n';
     std::cout << "float              " << can_alias_float           () << '\n';
 }

Again, discarding attribute may_alias is a semantic change,
because aliasing rules can affect code generation.

Why this issue is important?
Well, because it prevents generic programming, via C++ templates,
using x86 SIMD types.

If we would look at definitions of x86 SIMD types, we will notice that they are
essentially typedefs with attributes vector_size and may_alias applied on them:

- immintrin.h
   typedef float     __m256  __attribute__((__vector_size__(32), __may_alias__));

- emmintrin.h
   typedef long long __m128i __attribute__((__vector_size__(16), __may_alias__));
   typedef double    __m128d __attribute__((__vector_size__(16), __may_alias__));

- xmmintrin.h
   typedef float     __m128  __attribute__((__vector_size__(16), __may_alias__));

Note that, the may_alias attributes is required and cannot be removed:

- /usr/lib/gcc/x86_64-linux-gnu/10/include/immintrin.h
    /* The Intel API is flexible enough that we must allow aliasing with other
       vector types, and their scalar components.  */

What's the root cause of this problem?

Well, the problem is a C++ typedef is just an alias (a new name) for the old
type,
that is, it does *not* introduce a new type.

Implementing support for attributes vector_size, aligned and may_alias
in C++ typedefs requires an opaque/strong typedef,
introducing a brand new type and storing information about applied attributes.

 typedef float fp __attribute__((aligned(16)));

Think about it - a typedef introducing the fp type has to create a new type,
because even though both fp and float types represent
floating point numbers identically, the fp type is *not* the float type,
because these types have different alignment requirements.

Note that, the Intel C++ Compiler does not introduce new types for typedefs,
which have attributes aligned or may_alias applied on them.

Compiler Explorer:
 ICC -> https://godbolt.org/z/MjdMqx

C++ Source Code:
 #include <iostream>

 typedef int vectorized_int __attribute__((vector_size(8)));
 typedef int    aligned_int __attribute__((aligned(16)));
 typedef int   aliasing_int __attribute__((may_alias));

 int main ()
 {
     std::cout << typeid(           int).name() << std::endl;
     std::cout << typeid(vectorized_int).name() << std::endl;
     std::cout << typeid(   aligned_int).name() << std::endl;
     std::cout << typeid(  aliasing_int).name() << std::endl;
 }

Program Output:
 i
 Dv2_i
 i
 i

However, this behavior leads to a contradiction, in which
there can exists a single type, which has 2 different alignment requirements.

Compiler Explorer:
 ICC -> https://godbolt.org/z/4o9o3M

C++ Source Code:
 template <class, class> struct is_same       { static const auto value = false; };
 template <class T>      struct is_same<T, T> { static const auto value =  true; };

 typedef float fp __attribute__((aligned(16)));

 template <typename first_type, typename second_type>
 struct check_same
 {
     static_assert(is_same<first_type, second_type>::value    , "");
     static_assert( sizeof(first_type) ==  sizeof(second_type), "");
     static_assert(alignof(first_type) == alignof(second_type), "");
 };

 int main ()
 {
     check_same<int, signed int> { };
     check_same< fp,      float> { };
 }

Compilation Log:
 error: static assertion failed
     static_assert(alignof(first_type) == alignof(second_type), "");
     ^
 detected during instantiation of class
 "check_same<first_type, second_type> [with first_type=fp={float}, second_type=float]"

To avoid these kind of issues,
Clang could replicate the behavior of the vector_size attribute, that is,
introduce a brand new type and store information about applied attributes.

Thank you, Mateusz Zych

PS. I want to thank Ivo Kabadshow from JSC for helping me with
    preparing these code samples!
Quuxplusone commented 4 years ago
Since Clang aims to be compatible with GCC,
I want to mention that GCC also has this issue and I've already reported it:

 - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97222

Also, I hope this will allow GCC and Clang developers to figure out
how to resolve this issue, since (according to Andrew Pinski)
solving it requires addition to an Itanium ABI, used by both GCC and Clang.

Thanks, Mateusz
Quuxplusone commented 4 years ago
Thanks for the detailed writeup!

Your analysis is basically correct, but I would add that ICC's behavior here is
unsound (it only appears to "implement[] this behavior correctly" in simple
cases), whereas the GCC/Clang behavior is sound but surprising. For GCC, ICC,
and Clang, identity<fp> and identity<float> are the same type. Therefore it's
not possible for identity<fp>::type and identity<float>::type to have different
alignments, because they're the same type. So, for an example such as this:

    std::cout << alignof(typename identity<fp>::type) << std::endl;
    std::cout << alignof(typename identity<float>::type) << std::endl;

under GCC and Clang, both lines print out 4, whereas under ICC, they either
both print out 4 or both print out 16 depending on which one happens to appear
first in the program (and in general you can encounter ODR violations when
using ICC despite there being nothing wrong at the source level).

Fundamentally, the 'aligned' attribute is a GCC extension, so the GCC folks get
to define how it works. And they chose that instead of it resulting in a
different type that's *almost* like the original type (for example, treating it
as a type qualifier), it results in the same type, but that in some contexts
that same type behaves differently. That's a semantic disaster, but it's what
we live with. And in particular, the only way for template instantiation to be
sound in the presence of this semantic disaster is for it to ignore all such
attributes on template type arguments.

I think there are two things we could reasonably do here:

1) Implement the warning that modern versions of GCC produce when such a type
is used as a template argument, pointing out that the attribute that modifies
the type is ignored, and

2) Add alternative forms of the functionality question that behave like real
type qualifiers (as suggested). Note that we can't reasonably change the
meaning of __attribute__((aligned(N))) to do this; it'd need to be distinct
syntax.

The fact that our own intrin.h headers don't work reliably due to this issue
seems like a pretty good argument that we should do (2), for at least the
may_alias attribute.
Quuxplusone commented 4 years ago
Hi Richard,

Wow, I already was thinking that ICC behavior is contradictory,
because of this "single type, but two different alignments" issue,
but your argument makes it clear that ICC is not implementing correctly
typedefs with attribute aligned applied on them.

Compiler Explorer links with your examples:
 ICC   -> https://godbolt.org/z/z91axs
 Clang -> https://godbolt.org/z/4fP8Tb

I agree that ICC's lack of consistency is even worse than GCC and Clang
discarding the aligned attribute, since it leads to consistency issues.

Regarding the steps to improve current situation,
I agree that issuing a warning about ignored attributes would be good idea,
since it would make developers aware of what is actually happening.

Also, I understand that due to backward compatibility changing current
semantics of the aligned and may_alias attributes is not possible.
This is fine, as long as x86 SIMD headers could be updated in such a way,
that x86 SIMD types could be passed as template arguments safely.

Note that, some x86 SIMD types use both may_alias and aligned attributes:

 - /usr/lib/gcc/x86_64-linux-gnu/9/include/xmmintrin.h

    typedef float __m128   __attribute__ ((__vector_size__(16),
                                           __may_alias__));
    /* Unaligned version of the same type.  */
    typedef float __m128_u __attribute__ ((__vector_size__(16),
                                           __may_alias__,
                                           __aligned__(1)));

Since there is no standard mechanism, which could replicate the behavior of
the may_alias attribute, I agree that a new way of controlling aliasing rules
for a particular type might be necessary.

Unfortunately, we might also need a new way of specifying alignment,
since the alignas specifier from C++11 will not help here,
because it does not allow decreasing alignment requirements:

 "The combined effect of all alignment-specifiers in a declaration
  shall not specify an alignment that is less strict than
  the alignment that would be required for the entity being declared
  if all alignment-specifiers were omitted (including those
  in other declarations)."

On the other hand, the GCC's aligned attribute does explicitly allow it:

 "When used as part of a typedef, the aligned attribute can both
  increase and decrease alignment, and specifying the packed attribute
  generates a warning."

However, introducing new aliasing and alignment specifiers
would increase complexity of Clang and GCC, so maybe we could to avoid it?

Since all x86 SIMD types have attribute vector_size applied on them,
and the vector_size attribute always introduces a brand new type,
then could we simply change x86 SIMD header files to use structs?

Obviously such change would break code using C++ type_traits to detect
whether an x86 SIMD type is a class type (via std::is_class meta-function),
but this shouldn't be a huge problem is practice, right?

What do you think?
Thank you, Mateusz Zych