[multiple changes]

2014-11-12  Thomas Preud'homme  <thomas.preudhomme@arm.com>

    gcc/
    PR tree-optimization/63761
    * tree-ssa-math-opts.c (bswap_replace): Construct gsi from cur_stmt
    rather than taking it as a parameter. Add some comments to explain the
    gsi_move_before in case of load and why canonicalization of bswap into
    a rotation is only done for 16bit values.
    (pass_optimize_bswap::execute): Adapt for loop via gsi to make gsi
    refer to the statement just before cur_stmt. Ignore 16bit bswap that
    are already in canonical form. Adapt bswap_replace to removal of its
    gsi parameter.

    2014-11-12  Thomas Preud'homme  <thomas.preudhomme@arm.com>

    gcc/testsuite/
    PR tree-optimization/63761
    * gcc.c-torture/compile/pr63761.c: New test.

From-SVN: r217409

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index fc8510dc972adc4f4541744f42ef06b21519dbb0..d6c7d383db68a9f2a0ea3913e11ffecb181b52e3 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,15 @@
+2014-11-12  Thomas Preud'homme  <thomas.preudhomme@arm.com>
+
+       PR tree-optimization/63761
+       * tree-ssa-math-opts.c (bswap_replace): Construct gsi from cur_stmt
+       rather than taking it as a parameter. Add some comments to explain the
+       gsi_move_before in case of load and why canonicalization of bswap into
+       a rotation is only done for 16bit values.
+       (pass_optimize_bswap::execute): Adapt for loop via gsi to make gsi
+       refer to the statement just before cur_stmt. Ignore 16bit bswap that
+       are already in canonical form. Adapt bswap_replace to removal of its
+       gsi parameter.
+
 2014-11-12  Richard Sandiford  <richard.sandiford@arm.com>
 
        * rtl.h (rtx_function, for_each_rtx, for_each_rtx_in_insn): Delete.

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 1cb9e1cc36f21e671cde3bf6442eb7f5b5193fc5..10c4e6f9fafa193f8f1f23db66539070f2d92698 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-11-12  Thomas Preud'homme  <thomas.preudhomme@arm.com>
+
+       PR tree-optimization/63761
+       * gcc.c-torture/compile/pr63761.c: New test.
+
 2014-11-12  Jiong Wang  <jiong.wang@arm.com>
 
        * lib/gcc-dg.exp (${tool}_load): Truncate gcc output.

diff --git a/gcc/testsuite/gcc.c-torture/compile/pr63761.c b/gcc/testsuite/gcc.c-torture/compile/pr63761.c
new file mode 100644 (file)
index 0000000..5cda3f1
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr63761.c
@@ -0,0 +1,17 @@
+int a, b;
+short c;
+
+void fn1 ();
+
+void
+fn2 (unsigned short p1)
+{
+  int d;
+
+  c = p1 >> 8 | p1 << 8;
+  d = b;
+  if (d)
+    fn1 ();
+  a = d >> 8 & 0x00FF
+    | d << 8 & 0xFF00;
+}

diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index 12fa4e8ca10632e6b6bd519357dbf83582828de8..aab056c338e00aa0c0c9846fd5a24ee6639acf12 100644 (file)
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -2172,23 +2172,28 @@ public:
 
 }; // class pass_optimize_bswap
 
-/* Perform the bswap optimization: replace the statement CUR_STMT at
-   GSI with a load of type, VUSE and set-alias as described by N if a
-   memory source is involved (N->base_addr is non null), followed by
-   the builtin bswap invocation in FNDECL if BSWAP is true.  SRC_STMT
-   gives where should the replacement be made.  It also gives the
-   source on which CUR_STMT is operating via its rhs's first tree nad
-   N->range gives the size of the expression involved for maintaining
-   some statistics.  */
+/* Perform the bswap optimization: replace the expression computed in the rhs
+   of CUR_STMT by an equivalent bswap, load or load + bswap expression.
+   Which of these alternatives replace the rhs is given by N->base_addr (non
+   null if a load is needed) and BSWAP.  The type, VUSE and set-alias of the
+   load to perform are also given in N while the builtin bswap invoke is given
+   in FNDEL.  Finally, if a load is involved, SRC_STMT refers to one of the
+   load statements involved to construct the rhs in CUR_STMT and N->range gives
+   the size of the rhs expression for maintaining some statistics.
+
+   Note that if the replacement involve a load, CUR_STMT is moved just after
+   SRC_STMT to do the load with the same VUSE which can lead to CUR_STMT
+   changing of basic block.  */
 
 static bool
-bswap_replace (gimple cur_stmt, gimple_stmt_iterator gsi, gimple src_stmt,
-              tree fndecl, tree bswap_type, tree load_type,
-              struct symbolic_number *n, bool bswap)
+bswap_replace (gimple cur_stmt, gimple src_stmt, tree fndecl, tree bswap_type,
+              tree load_type, struct symbolic_number *n, bool bswap)
 {
+  gimple_stmt_iterator gsi;
   tree src, tmp, tgt;
   gimple bswap_stmt;
 
+  gsi = gsi_for_stmt (cur_stmt);
   src = gimple_assign_rhs1 (src_stmt);
   tgt = gimple_assign_lhs (cur_stmt);
 
@@ -2207,6 +2212,9 @@ bswap_replace (gimple cur_stmt, gimple_stmt_iterator gsi, gimple src_stmt,
          && SLOW_UNALIGNED_ACCESS (TYPE_MODE (load_type), align))
        return false;
 
+      /* Move cur_stmt just before  one of the load of the original
+        to ensure it has the same VUSE.  See PR61517 for what could
+        go wrong.  */
       gsi_move_before (&gsi, &gsi_ins);
       gsi = gsi_for_stmt (cur_stmt);
 
@@ -2293,7 +2301,10 @@ bswap_replace (gimple cur_stmt, gimple_stmt_iterator gsi, gimple src_stmt,
 
   tmp = src;
 
-  /* Canonical form for 16 bit bswap is a rotate expression.  */
+  /* Canonical form for 16 bit bswap is a rotate expression.  Only 16bit values
+     are considered as rotation of 2N bit values by N bits is generally not
+     equivalent to a bswap.  Consider for instance 0x01020304 >> 16 which gives
+     0x03040102 while a bswap for that value is 0x04030201.  */
   if (bswap && n->range == 16)
     {
       tree count = build_int_cst (NULL, BITS_PER_UNIT);
@@ -2393,10 +2404,10 @@ pass_optimize_bswap::execute (function *fun)
       gimple_stmt_iterator gsi;
 
       /* We do a reverse scan for bswap patterns to make sure we get the
-        widest match. As bswap pattern matching doesn't handle
-        previously inserted smaller bswap replacements as sub-
-        patterns, the wider variant wouldn't be detected.  */
-      for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi))
+        widest match. As bswap pattern matching doesn't handle previously
+        inserted smaller bswap replacements as sub-patterns, the wider
+        variant wouldn't be detected.  */
+      for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi);)
         {
          gimple src_stmt, cur_stmt = gsi_stmt (gsi);
          tree fndecl = NULL_TREE, bswap_type = NULL_TREE, load_type;
@@ -2404,6 +2415,14 @@ pass_optimize_bswap::execute (function *fun)
          struct symbolic_number n;
          bool bswap;
 
+         /* This gsi_prev (&gsi) is not part of the for loop because cur_stmt
+            might be moved to a different basic block by bswap_replace and gsi
+            must not points to it if that's the case.  Moving the gsi_prev
+            there make sure that gsi points to the statement previous to
+            cur_stmt while still making sure that all statements are
+            considered in this basic block.  */
+         gsi_prev (&gsi);
+
          if (!is_gimple_assign (cur_stmt))
            continue;
 
@@ -2431,6 +2450,9 @@ pass_optimize_bswap::execute (function *fun)
          switch (n.range)
            {
            case 16:
+             /* Already in canonical form, nothing to do.  */
+             if (code == LROTATE_EXPR || code == RROTATE_EXPR)
+               continue;
              load_type = uint16_type_node;
              if (bswap16_p)
                {
@@ -2461,8 +2483,8 @@ pass_optimize_bswap::execute (function *fun)
          if (bswap && !fndecl)
            continue;
 
-         if (bswap_replace (cur_stmt, gsi, src_stmt, fndecl, bswap_type,
-                            load_type, &n, bswap))
+         if (bswap_replace (cur_stmt, src_stmt, fndecl, bswap_type, load_type,
+                            &n, bswap))
            changed = true;
        }
     }