Bug 20982

Summary: alignment attribute ignired for vector pointers types
Product: gcc Reporter: Håkon Bugge <haakon.bugge>
Component: middle-endAssignee: Not yet assigned to anyone <unassigned>
Status: NEW ---    
Severity: normal CC: gcc-bugs, manu
Priority: P2 Keywords: diagnostic
Version: 3.4.3   
Target Milestone: ---   
Host: Target: i686-pc-linux-gnu
Build: Known to work:
Known to fail: Last reconfirmed: 2009-02-07 20:46:55

Description Håkon Bugge 2005-04-12 23:34:00 UTC
creating a vector type aligned on a smaller unit than the size of the vector, 
produces incorrect SSE code. On Intel SSE, movups must be used instead of movaps, 
and memory operands to other SSE instructons must be avoided.

E.g. :

typedef float v4sf __attribute__ ((mode(V4SF),aligned(8)));

v4sf foo(v4sf *p, v4sf *q) {
  return *p + *q;
}

produces

   0:   0f 28 07                movaps (%rdi),%xmm0
   3:   0f 58 06                addps  (%rsi),%xmm0
   6:   c3                      retq   

it should have been something like:

   movups (%rdi),%xmm0
   movups (%rsi),%xmm1
   addps  %xmm1, %xmm0
   retq

Thanks, Håkon
Comment 1 Andrew Pinski 2005-04-12 23:36:33 UTC
aligned can never decrease the required alignment.
Comment 2 Manuel López-Ibáñez 2007-11-16 19:32:53 UTC
(In reply to comment #1)
> aligned can never decrease the required alignment.
> 

Can we detect this and emit a diagnostic? We do it for declarations already but not for types. I tried the following patch as a wild guess but it generates regressions in gcc.dg/compat/struct-layout-1 and similar.

Index: gcc/testsuite/gcc.dg/pr20982.c
===================================================================
--- gcc/testsuite/gcc.dg/pr20982.c      (revision 0)
+++ gcc/testsuite/gcc.dg/pr20982.c      (revision 0)
@@ -0,0 +1,11 @@
+/* PR 20982 alignment attribute ignored for vector pointers types */
+/* { dg-do compile } */
+/* { dg-options "-msse " } */
+typedef float v4sf __attribute__ ((mode(V4SF),aligned(8)));
+/* { dg-warning "specifying vector types with __attribute__ ..mode.. is deprecated" } "" { target *-*-* } 4 }*/
+/* { dg-warning " use .* instead" "" { target *-*-* } 4 } */
+/* { dg-error "alignment for .* must be at least" "" { target *-*-* } 4 } */
+
+v4sf foo(v4sf *p, v4sf *q) {
+  return *p + *q;
+}
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c      (revision 130174)
+++ gcc/c-common.c      (working copy)
@@ -5438,23 +5438,38 @@
     }
   else if (is_type)
     {
-      /* If we have a TYPE_DECL, then copy the type, so that we
-        don't accidentally modify a builtin type.  See pushdecl.  */
-      if (decl && TREE_TYPE (decl) != error_mark_node
-         && DECL_ORIGINAL_TYPE (decl) == NULL_TREE)
-       {
-         tree tt = TREE_TYPE (decl);
-         *type = build_variant_type_copy (*type);
-         DECL_ORIGINAL_TYPE (decl) = tt;
-         TYPE_NAME (*type) = decl;
-         TREE_USED (*type) = TREE_USED (decl);
-         TREE_TYPE (decl) = *type;
-       }
-      else if (!(flags & (int) ATTR_FLAG_TYPE_IN_PLACE))
-       *type = build_variant_type_copy (*type);
-
-      TYPE_ALIGN (*type) = (1 << i) * BITS_PER_UNIT;
-      TYPE_USER_ALIGN (*type) = 1;
+      if (TYPE_ALIGN (*type) > (unsigned) (1 << i) * BITS_PER_UNIT)
+        {
+          if (TYPE_USER_ALIGN (*type))
+            error ("alignment for %qT was previously specified as %d "
+                   "and may not be decreased", *type,
+                   TYPE_ALIGN (*type) / BITS_PER_UNIT);
+          else
+            error ("alignment for %qT must be at least %d", *type,
+                   TYPE_ALIGN (*type) / BITS_PER_UNIT);
+          *no_add_attrs = true;
+        }
+      else
+        {
+          /* If we have a TYPE_DECL, then copy the type, so that we
+             don't accidentally modify a builtin type.  See pushdecl.  */
+          if (decl && TREE_TYPE (decl) != error_mark_node
+              && DECL_ORIGINAL_TYPE (decl) == NULL_TREE)
+            {
+              tree tt = TREE_TYPE (decl);
+              *type = build_variant_type_copy (*type);
+              DECL_ORIGINAL_TYPE (decl) = tt;
+              TYPE_NAME (*type) = decl;
+              TREE_USED (*type) = TREE_USED (decl);
+              TREE_TYPE (decl) = *type;
+            }
+          else if (!(flags & (int) ATTR_FLAG_TYPE_IN_PLACE))
+            *type = build_variant_type_copy (*type);
+
+
+          TYPE_ALIGN (*type) = (1 << i) * BITS_PER_UNIT;
+          TYPE_USER_ALIGN (*type) = 1;
+        }
     }
   else if (! VAR_OR_FUNCTION_DECL_P (decl)
           && TREE_CODE (decl) != FIELD_DECL)
Comment 3 Manuel López-Ibáñez 2009-02-07 20:46:55 UTC
This is a missing diagnostic.