public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR63266: Keep track of impact of sign extension in bswap
@ 2014-09-16 10:27 Thomas Preud'homme
  2014-09-24  1:57 ` Thomas Preud'homme
  2014-09-24  8:01 ` Richard Biener
  0 siblings, 2 replies; 20+ messages in thread
From: Thomas Preud'homme @ 2014-09-16 10:27 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1513 bytes --]

Hi all,

The fix for PR61306 disabled bswap when a sign extension is detected. However this led to a test case regression (and potential performance regression) in case where a sign extension happens but its effect is canceled by other bit manipulation. This patch aims to fix that by having a special marker to track bytes whose value is unpredictable due to sign extension. If the final result of a bit manipulation doesn't contain any such marker then the bswap optimization can proceed.

*** gcc/ChangeLog ***

2014-09-15  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR tree-optimization/63266
	* tree-ssa-math-opts.c (struct symbolic_number): Add comment about
	marker for unknown byte value.
	(MARKER_MASK): New macro.
	(MARKER_BYTE_UNKNOWN): New macro.
	(HEAD_MARKER): New macro.
	(do_shift_rotate): Mark bytes with unknown values due to sign
	extension when doing an arithmetic right shift. Replace hardcoded
	mask for marker by new MARKER_MASK macro.
	(find_bswap_or_nop_1): Likewise and adjust ORing of two symbolic
	numbers accordingly.

*** gcc/testsuite/ChangeLog ***

2014-09-15  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR tree-optimization/63266
	* gcc.dg/optimize-bswapsi-1.c (swap32_d): New bswap pass test.


Testing:

* Built an arm-none-eabi-gcc cross-compiler and used it to run the testsuite on QEMU emulating Cortex-M3 without any regression
* Bootstrapped on x86_64-linux-gnu target and testsuite was run without regression


Ok for trunk?

[-- Attachment #2: pr63266.1.1.diff --]
[-- Type: application/octet-stream, Size: 5706 bytes --]

diff --git a/gcc/testsuite/gcc.dg/optimize-bswapsi-1.c b/gcc/testsuite/gcc.dg/optimize-bswapsi-1.c
index ebfca60..580e6e0 100644
--- a/gcc/testsuite/gcc.dg/optimize-bswapsi-1.c
+++ b/gcc/testsuite/gcc.dg/optimize-bswapsi-1.c
@@ -49,5 +49,20 @@ swap32_c (SItype u)
 	  | (((u) & 0x000000ff) << 24));
 }
 
-/* { dg-final { scan-tree-dump-times "32 bit bswap implementation found at" 3 "bswap" } } */
+/* This variant comes from gcc.target/sh/pr53568-1.c.  It requires to track
+   which bytes have an unpredictable value (eg. due to sign extension) to
+   make sure that the final expression have only well defined byte values.  */
+
+SItype
+swap32_d (SItype in)
+{
+  /* 1x swap.w
+     2x swap.b  */
+  return (((in >> 0) & 0xFF) << 24)
+	 | (((in >> 8) & 0xFF) << 16)
+	 | (((in >> 16) & 0xFF) << 8)
+	 | (((in >> 24) & 0xFF) << 0);
+}
+
+/* { dg-final { scan-tree-dump-times "32 bit bswap implementation found at" 4 "bswap" } } */
 /* { dg-final { cleanup-tree-dump "bswap" } } */
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index c1c2c82..3c6e935 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -1603,6 +1603,7 @@ make_pass_cse_sincos (gcc::context *ctxt)
    consisting of octet sized markers:
 
    0    - target byte has the value 0
+   FF   - target byte has an unknown value (eg. due to sign extension)
    1..size - marker value is the target byte index minus one.
 
    To detect permutations on memory sources (arrays and structures), a symbolic
@@ -1629,6 +1630,10 @@ struct symbolic_number {
 };
 
 #define BITS_PER_MARKER 8
+#define MARKER_MASK ((1 << BITS_PER_MARKER) - 1)
+#define MARKER_BYTE_UNKNOWN MARKER_MASK
+#define HEAD_MARKER(n, size) \
+  ((n) & ((uint64_t) MARKER_MASK << (((size) - 1) * BITS_PER_MARKER)))
 
 /* The number which the find_bswap_or_nop_1 result should match in
    order to have a nop.  The number is masked according to the size of
@@ -1651,7 +1656,8 @@ do_shift_rotate (enum tree_code code,
 		 struct symbolic_number *n,
 		 int count)
 {
-  int size = TYPE_PRECISION (n->type) / BITS_PER_UNIT;
+  int i, size = TYPE_PRECISION (n->type) / BITS_PER_UNIT;
+  unsigned head_marker;
 
   if (count % BITS_PER_UNIT != 0)
     return false;
@@ -1668,11 +1674,13 @@ do_shift_rotate (enum tree_code code,
       n->n <<= count;
       break;
     case RSHIFT_EXPR:
-      /* Arithmetic shift of signed type: result is dependent on the value.  */
-      if (!TYPE_UNSIGNED (n->type)
-	  && (n->n & ((uint64_t) 0xff << ((size - 1) * BITS_PER_MARKER))))
-	return false;
+      head_marker = HEAD_MARKER (n->n, size);
       n->n >>= count;
+      /* Arithmetic shift of signed type: result is dependent on the value.  */
+      if (!TYPE_UNSIGNED (n->type) && head_marker)
+	for (i = 0; i < count / BITS_PER_MARKER; i++)
+	  n->n |= (uint64_t) MARKER_BYTE_UNKNOWN
+		  << ((size - 1 - i) * BITS_PER_MARKER);
       break;
     case LROTATE_EXPR:
       n->n = (n->n << count) | (n->n >> ((size * BITS_PER_MARKER) - count));
@@ -1878,7 +1886,7 @@ find_bswap_or_nop_1 (gimple stmt, struct symbolic_number *n, int limit)
 	      if ((val & tmp) != 0 && (val & tmp) != tmp)
 		return NULL;
 	      else if (val & tmp)
-		mask |= (uint64_t) 0xff << (i * BITS_PER_MARKER);
+		mask |= (uint64_t) MARKER_MASK << (i * BITS_PER_MARKER);
 
 	    n->n &= mask;
 	  }
@@ -1892,7 +1900,7 @@ find_bswap_or_nop_1 (gimple stmt, struct symbolic_number *n, int limit)
 	  break;
 	CASE_CONVERT:
 	  {
-	    int type_size, old_type_size;
+	    int i, type_size, old_type_size;
 	    tree type;
 
 	    type = gimple_expr_type (stmt);
@@ -1905,11 +1913,10 @@ find_bswap_or_nop_1 (gimple stmt, struct symbolic_number *n, int limit)
 
 	    /* Sign extension: result is dependent on the value.  */
 	    old_type_size = TYPE_PRECISION (n->type) / BITS_PER_UNIT;
-	    if (!TYPE_UNSIGNED (n->type)
-		&& type_size > old_type_size
-		&& n->n & ((uint64_t) 0xff << ((old_type_size - 1)
-					       * BITS_PER_MARKER)))
-	      return NULL;
+	    if (!TYPE_UNSIGNED (n->type) && type_size > old_type_size
+		&& HEAD_MARKER (n->n, old_type_size))
+	      for (i = 0; i < type_size - old_type_size; i++)
+		n->n |= MARKER_BYTE_UNKNOWN << (type_size - 1 - i);
 
 	    if (type_size < 64 / BITS_PER_MARKER)
 	      {
@@ -1968,7 +1975,7 @@ find_bswap_or_nop_1 (gimple stmt, struct symbolic_number *n, int limit)
 	  if (gimple_assign_rhs1 (source_stmt1)
 	      != gimple_assign_rhs1 (source_stmt2))
 	    {
-	      int64_t inc, mask;
+	      int64_t inc;
 	      HOST_WIDE_INT off_sub;
 	      struct symbolic_number *n_ptr;
 
@@ -2002,15 +2009,15 @@ find_bswap_or_nop_1 (gimple stmt, struct symbolic_number *n, int limit)
 		 bigger weight according to target endianness.  */
 	      inc = BYTES_BIG_ENDIAN ? off_sub + n2.range - n1.range : off_sub;
 	      size = TYPE_PRECISION (n1.type) / BITS_PER_UNIT;
-	      mask = 0xff;
 	      if (BYTES_BIG_ENDIAN)
 		n_ptr = &n1;
 	      else
 		n_ptr = &n2;
-	      for (i = 0; i < size; i++, inc <<= BITS_PER_MARKER,
-					 mask <<= BITS_PER_MARKER)
+	      for (i = 0; i < size; i++, inc <<= BITS_PER_MARKER)
 		{
-		  if (n_ptr->n & mask)
+		  unsigned marker =
+		    (n_ptr->n >> (i * BITS_PER_MARKER)) & MARKER_MASK;
+		  if (marker && marker != MARKER_BYTE_UNKNOWN)
 		    n_ptr->n += inc;
 		}
 	    }
@@ -2028,7 +2035,8 @@ find_bswap_or_nop_1 (gimple stmt, struct symbolic_number *n, int limit)
 	  n->bytepos = n1.bytepos;
 	  n->type = n1.type;
 	  size = TYPE_PRECISION (n->type) / BITS_PER_UNIT;
-	  for (i = 0, mask = 0xff; i < size; i++, mask <<= BITS_PER_MARKER)
+	  for (i = 0, mask = MARKER_MASK; i < size;
+	       i++, mask <<= BITS_PER_MARKER)
 	    {
 	      uint64_t masked1, masked2;
 

^ permalink raw reply	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2014-10-29 10:35 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-16 10:27 [PATCH] Fix PR63266: Keep track of impact of sign extension in bswap Thomas Preud'homme
2014-09-24  1:57 ` Thomas Preud'homme
2014-09-24  8:01 ` Richard Biener
2014-09-24 20:27   ` Christophe Lyon
2014-09-25  6:41     ` Thomas Preud'homme
2014-09-25 14:08       ` Christophe Lyon
2014-09-26  2:27         ` Thomas Preud'homme
2014-09-26 16:12           ` Christophe Lyon
2014-10-21  9:30   ` Thomas Preud'homme
2014-10-21 11:50     ` Richard Biener
2014-10-21 21:06     ` Christophe Lyon
2014-10-22  8:57       ` Thomas Preud'homme
2014-10-26 16:50         ` Christophe Lyon
2014-10-27 12:16           ` Thomas Preud'homme
2014-10-28 12:28     ` [PATCH] Fix up " Jakub Jelinek
2014-10-28 13:00       ` Richard Biener
2014-10-29  9:36       ` Thomas Preud'homme
2014-10-29  9:40         ` Thomas Preud'homme
2014-10-29  9:43           ` Jakub Jelinek
2014-10-29 10:37             ` Thomas Preud'homme

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).