Bug 20982 - alignment attribute ignired for vector pointers types
Summary: alignment attribute ignired for vector pointers types
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 3.4.3
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2005-04-12 23:33 UTC by Håkon Bugge
Modified: 2009-02-07 20:46 UTC (History)
2 users (show)

See Also:
Host:
Target: i686-pc-linux-gnu
Build:
Known to work:
Known to fail:
Last reconfirmed: 2009-02-07 20:46:55


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.