This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix PR69264


This fixes PR69264, reverting an earlier change that was trying to
bypass the broken/unclear vector_alignment_reachable hook.  x86
doesn't define this hook and thus inherits the default implementation.

Now - the args we feed to the hook changed over time, esp. the
is_packed arg now (correctly) tells the hook about the alignment
of the access and whether it is naturally aligned (according to its size).

The hook was originally added for power where alignment of double
is 32bits but vector double requires 128bit alignment and thus peeling
might never reach proper aligned vector accesses.  Nowadays the
default implementation of the hook already ensures proper behavior
here but it also contains weird code from the times is_packed was
just TYPE_PACKED of the access.

So the following simplifies it up to the point that I know no target
that isn't happy with the default implementation which all targets
can use to get conservative correct behavior.

But at this point I didn't want to remove the hook (targets can
individually remove theirs please -- comments in most of those
hooks tell me they didn't really understand the purpose of the
hook, thus I clarified its docs).

Similar misconceptions may be in the related support_vector_misalignment
hook (for the is_packed handling for targets requiring element aligned
vectors only for example).

Bootstrap / regtest running on x86_64-unknown-linux-gnu.

Richard.

2017-01-25  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/69264
	* target.def (vector_alignment_reachable): Improve documentation.
	* targhooks.c (default_builtin_vector_alignment_reachable): Simplify
	and add a comment.
	* tree-vect-data-refs.c (vect_supportable_dr_alignment): Revert
	earlier changes with respect to TYPE_USER_ALIGN.
	(vector_alignment_reachable_p): Likewise.  Improve dumping.

	* g++.dg/torture/pr69264.C: New testcase.

Index: gcc/target.def
===================================================================
--- gcc/target.def	(revision 244890)
+++ gcc/target.def	(working copy)
@@ -1801,10 +1801,10 @@ misalignment value (@var{misalign}).",
  default_builtin_vectorization_cost)
 
 /* Return true if vector alignment is reachable (by peeling N
-   iterations) for the given type.  */
+   iterations) for the given scalar type.  */
 DEFHOOK
 (vector_alignment_reachable,
- "Return true if vector alignment is reachable (by peeling N iterations) for the given type.",
+ "Return true if vector alignment is reachable (by peeling N iterations) for the given scalar type @var{type}.  @var{is_packed} is false if the scalar access using @var{type} is known to be naturally aligned.",
  bool, (const_tree type, bool is_packed),
  default_builtin_vector_alignment_reachable)
 
Index: gcc/targhooks.c
===================================================================
--- gcc/targhooks.c	(revision 244890)
+++ gcc/targhooks.c	(working copy)
@@ -1127,20 +1127,12 @@ default_vector_alignment (const_tree typ
   return align;
 }
 
+/* By default assume vectors of element TYPE require a multiple of the natural
+   alignment of TYPE.  TYPE is naturally aligned if IS_PACKED is false.  */
 bool
-default_builtin_vector_alignment_reachable (const_tree type, bool is_packed)
+default_builtin_vector_alignment_reachable (const_tree /*type*/, bool is_packed)
 {
-  if (is_packed)
-    return false;
-
-  /* Assuming that types whose size is > pointer-size are not guaranteed to be
-     naturally aligned.  */
-  if (tree_int_cst_compare (TYPE_SIZE (type), bitsize_int (POINTER_SIZE)) > 0)
-    return false;
-
-  /* Assuming that types whose size is <= pointer-size
-     are naturally aligned.  */
-  return true;
+  return ! is_packed;
 }
 
 /* By default, assume that a target supports any factor of misalignment
Index: gcc/tree-vect-data-refs.c
===================================================================
--- gcc/tree-vect-data-refs.c	(revision 244890)
+++ gcc/tree-vect-data-refs.c	(working copy)
@@ -1098,12 +1098,9 @@ vector_alignment_reachable_p (struct dat
       bool is_packed = not_size_aligned (DR_REF (dr));
       if (dump_enabled_p ())
 	dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
-	                 "Unknown misalignment, is_packed = %d\n",is_packed);
-      if ((TYPE_USER_ALIGN (type) && !is_packed)
-	  || targetm.vectorize.vector_alignment_reachable (type, is_packed))
-	return true;
-      else
-	return false;
+	                 "Unknown misalignment, %snaturally aligned\n",
+			 is_packed ? "not " : "");
+      return targetm.vectorize.vector_alignment_reachable (type, is_packed);
     }
 
   return true;
@@ -6153,10 +6150,8 @@ vect_supportable_dr_alignment (struct da
       if (!known_alignment_for_access_p (dr))
 	is_packed = not_size_aligned (DR_REF (dr));
 
-      if ((TYPE_USER_ALIGN (type) && !is_packed)
-	  || targetm.vectorize.
-	       support_vector_misalignment (mode, type,
-					    DR_MISALIGNMENT (dr), is_packed))
+      if (targetm.vectorize.support_vector_misalignment
+	    (mode, type, DR_MISALIGNMENT (dr), is_packed))
 	/* Can't software pipeline the loads, but can at least do them.  */
 	return dr_unaligned_supported;
     }
@@ -6168,10 +6163,8 @@ vect_supportable_dr_alignment (struct da
       if (!known_alignment_for_access_p (dr))
 	is_packed = not_size_aligned (DR_REF (dr));
 
-     if ((TYPE_USER_ALIGN (type) && !is_packed)
-	 || targetm.vectorize.
-	      support_vector_misalignment (mode, type,
-					   DR_MISALIGNMENT (dr), is_packed))
+     if (targetm.vectorize.support_vector_misalignment
+	   (mode, type, DR_MISALIGNMENT (dr), is_packed))
        return dr_unaligned_supported;
     }
 
Index: gcc/testsuite/g++.dg/torture/pr69264.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr69264.C	(nonexistent)
+++ gcc/testsuite/g++.dg/torture/pr69264.C	(working copy)
@@ -0,0 +1,81 @@
+// { dg-do compile }
+// { dg-additional-options "-mcpu=970 -maltivec" { target powerpc*-*-* } }
+
+typedef union {
+    long int asBits;
+} jsval_layout;
+static jsval_layout STRING_TO_JSVAL_IMPL() {}
+
+typedef __attribute__ ((aligned(sizeof (long int)))) long int jsval;
+class Value {
+public:
+    void setString() {
+	data = STRING_TO_JSVAL_IMPL();
+    }
+    jsval_layout data;
+} __attribute__ ((aligned(8)));
+
+static Value StringValue()
+{
+  Value v;
+  v.setString();
+  return v;
+}
+
+static const jsval & Jsvalify(const Value & v)
+{
+  return (const jsval &)v;
+}
+
+static Value *Valueify(jsval *v)
+{
+  return (Value *) v;
+}
+
+struct JSObject {
+    void getQNameLocalName();
+};
+static Value IdToValue(int id)
+{
+  if (id)
+    return StringValue();
+}
+
+static jsval IdToJsval(int id)
+{
+  return Jsvalify(IdToValue(id));
+}
+
+class AutoGCRooter;
+struct JSContext {
+    AutoGCRooter *autoGCRooters;
+};
+class AutoGCRooter {
+public:
+    AutoGCRooter(JSContext *cx) {}
+};
+class AutoArrayRooter:AutoGCRooter {
+public:
+    AutoArrayRooter(JSContext *cx, Value *vec):AutoGCRooter(cx)
+    {
+      array = vec;
+      cx->autoGCRooters = this;
+    }
+    Value *array;
+};
+
+static void PutProperty(JSContext *cx, int id, jsval *vp)
+{
+  JSObject *nameobj;
+  jsval roots[3];
+  roots[1] = IdToJsval(id);
+  roots[2] = *vp;
+  AutoArrayRooter tvr(cx, Valueify(roots));
+  nameobj->getQNameLocalName();
+}
+
+void xml_defineProperty(JSContext *cx, int id, const Value *v)
+{
+  jsval tmp = Jsvalify(*v);
+  PutProperty(cx, id, &tmp);
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]