This is the mail archive of the gcc-bugs@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]

[Bug target/69946] [6 Regression] Invalid ppc64 assembly emitted starting with r226005


https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69946

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
To me this looks like the bug is in the analysis function not matching what the
output function does.
rs6000_is_valid_shift_mask
has code like:
17346     /* Convert any shift by 0 to a rotate, to simplify below code.  */
17347     if (sh == 0)
17348       code = ROTATE;
17349   
17350     /* Convert rotate to simple shift if we can, to make analysis
simpler.  */
17351     if (code == ROTATE && sh >= 0 && nb >= ne && ne >= sh)
17352       code = ASHIFT;
17353     if (code == ROTATE && sh >= 0 && nb >= ne && nb < sh)
17354       {
17355         code = LSHIFTRT;
17356         sh = n - sh;
17357       }
but rs6000_insn_for_shift_mask doesn't do any of those transformations (neither
shifts to ROTATE, nor ROTATE to ASHIFT, nor ROTATE to LSHIFTRT.
The last one is what triggers on the testcase.
If I try to match what the analysis function does more closely:
--- gcc/config/rs6000/rs6000.c.jj       2016-02-12 00:50:55.000000000 +0100
+++ gcc/config/rs6000/rs6000.c  2016-02-24 18:55:19.945977868 +0100
@@ -17403,9 +17403,32 @@ rs6000_insn_for_shift_mask (machine_mode
   if (!rs6000_is_valid_mask (operands[3], &nb, &ne, mode))
     gcc_unreachable ();

+  int n = GET_MODE_PRECISION (mode);
+  int sh = -1;
+
+  if (CONST_INT_P (operands[2]))
+    sh = INTVAL (operands[2]);
+
+  rtx_code code = GET_CODE (operands[4]);
+
+  /* Convert any shift by 0 to a rotate, to match what the analysis
+     code did.  */
+  if (sh == 0)
+    code = ROTATE;
+
+  /* Convert rotate to simple shift if we can.  */
+  if (code == ROTATE && sh >= 0 && nb >= ne && ne >= sh)
+    code = ASHIFT;
+  if (code == ROTATE && sh >= 0 && nb >= ne && nb < sh)
+    {
+      code = LSHIFTRT;
+      sh = n - sh;
+      operands[2] = GEN_INT (sh);
+    }
+
   if (mode == DImode && ne == 0)
     {
-      if (GET_CODE (operands[4]) == LSHIFTRT && INTVAL (operands[2]))
+      if (code == LSHIFTRT && INTVAL (operands[2]))
        operands[2] = GEN_INT (64 - INTVAL (operands[2]));
       operands[3] = GEN_INT (63 - nb);
       if (dot)
@@ -17422,7 +17445,7 @@ rs6000_insn_for_shift_mask (machine_mode
     }

   if (mode == DImode
-      && GET_CODE (operands[4]) != LSHIFTRT
+      && code != LSHIFTRT
       && CONST_INT_P (operands[2])
       && ne == INTVAL (operands[2]))
     {
@@ -17434,7 +17457,7 @@ rs6000_insn_for_shift_mask (machine_mode

   if (nb < 32 && ne < 32)
     {
-      if (GET_CODE (operands[4]) == LSHIFTRT && INTVAL (operands[2]))
+      if (code == LSHIFTRT && INTVAL (operands[2]))
        operands[2] = GEN_INT (32 - INTVAL (operands[2]));
       operands[3] = GEN_INT (31 - nb);
       operands[4] = GEN_INT (31 - ne);

then the change in generated assembly is:
-       rlwinm 3,3,44,20,23
+       rlwinm 3,3,12,20,23
which I bet is correct.  If this is the way to go, then
rs6000_insn_for_insert_mask has similar problem (at least, doesn't match the
analysis function, whether it actually makes any difference for that or not is
unclear; but for this one we have a proof).

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