Hi!
Segher has added last year a few routines for the shift/rotate + mask
patterns, insns always have one predicate which tests if PowerPC supports
such pattern, and another that emits the instruction for it.
The testcase in the patch is miscompiled, we end up with an instruction
with out of bound shift count, because there is disagreement between the
analysis phase, which does some changes (changes shifts by 0
into rotate and some rotates into left or right shifts (for the last one
with changed shift count)), but those changes are only done virtually,
the predicate can't change the instruction, it either accepts it or rejects
it. Then during output, we don't perform those changes, treat it as
rotate or left or right shift just based on what the actual IL has.
In some cases it is harmless, in other cases, as the testcase shows, it is
harmful.
Also, I've noticed that the preparation phase of both
rs6000_is_valid_shift_mask and rs6000_is_valid_insert_mask is pretty much
the same (the only difference is that the latter requires the shift/rotate
count to be always CONST_INT, while the former allows also a REG in there).
So, what the following patch does is that it moves the preparation
from the predicate functions to a new static inline helper function,
and then uses that function in both the predicate functions, and also
in both the output functions.
Bootstrapped/regtested on powerpc64{,le}-linux, ok for trunk?
2016-02-26 Jakub Jelinek <ja...@redhat.com>
PR target/69946
* config/rs6000/rs6000.c (rs6000_is_valid_shift_mask_1): New
function.
(rs6000_is_valid_shift_mask, rs6000_is_valid_insert_mask): Use it.
(rs6000_insn_for_shift_mask, rs6000_insn_for_insert_mask): Likewise.
Adjust for possible changes of shift/rotate CODE and shift count SH.
* gcc.dg/pr69946.c: New test.
--- gcc/config/rs6000/rs6000.c.jj 2016-02-24 22:33:32.000000000 +0100
+++ gcc/config/rs6000/rs6000.c 2016-02-26 12:05:39.489021521 +0100
@@ -17319,42 +17319,60 @@ rs6000_insn_for_and_mask (machine_mode m
gcc_unreachable ();
}
-/* Return whether MASK (a CONST_INT) is a valid mask for any rlw[i]nm,
- rld[i]cl, rld[i]cr, or rld[i]c instruction, to implement an AND with
- shift SHIFT (a ROTATE, ASHIFT, or LSHIFTRT) in mode MODE. */
+/* Helper for rs6000_is_valid_shift_mask, rs6000_insn_for_shift_mask,
+ rs6000_is_valid_insert_mask and rs6000_insn_for_insert_mask. */
-bool
-rs6000_is_valid_shift_mask (rtx mask, rtx shift, machine_mode mode)
+static inline bool
+rs6000_is_valid_shift_mask_1 (rtx mask, rtx shift, machine_mode mode,
+ int *nb, int *ne, int *n, int *sh,
+ rtx_code *code)
{
- int nb, ne;
-
- if (!rs6000_is_valid_mask (mask, &nb, &ne, mode))
+ if (!rs6000_is_valid_mask (mask, nb, ne, mode))
return false;
- int n = GET_MODE_PRECISION (mode);
- int sh = -1;
+ *n = GET_MODE_PRECISION (mode);
+ *sh = -1;
if (CONST_INT_P (XEXP (shift, 1)))
{
- sh = INTVAL (XEXP (shift, 1));
- if (sh < 0 || sh >= n)
+ *sh = INTVAL (XEXP (shift, 1));
+ if (*sh < 0 || *sh >= *n)
return false;
}
- rtx_code code = GET_CODE (shift);
+ *code = GET_CODE (shift);
/* Convert any shift by 0 to a rotate, to simplify below code. */
- if (sh == 0)
- code = ROTATE;
+ if (*sh == 0)
+ *code = ROTATE;
/* Convert rotate to simple shift if we can, to make analysis simpler. */
- if (code == ROTATE && sh >= 0 && nb >= ne && ne >= sh)
- code = ASHIFT;
- if (code == ROTATE && sh >= 0 && nb >= ne && nb < sh)
+ if (*code == ROTATE && *sh >= 0 && *nb >= *ne)
{
- code = LSHIFTRT;
- sh = n - sh;
+ if (*ne >= *sh)
+ *code = ASHIFT;
+ else if (*nb < *sh)
+ {
+ *code = LSHIFTRT;
+ *sh = *n - *sh;
+ }
}
+ return true;
+}
+
+/* Return whether MASK (a CONST_INT) is a valid mask for any rlw[i]nm,
+ rld[i]cl, rld[i]cr, or rld[i]c instruction, to implement an AND with
+ shift SHIFT (a ROTATE, ASHIFT, or LSHIFTRT) in mode MODE. */
+
+bool
+rs6000_is_valid_shift_mask (rtx mask, rtx shift, machine_mode mode)
+{
+ int nb, ne, n, sh;
+ rtx_code code;
+
+ if (!rs6000_is_valid_shift_mask_1 (mask, shift, mode, &nb, &ne,
+ &n, &sh, &code))
+ return false;
/* DImode rotates need rld*. */
if (mode == DImode && code == ROTATE)
@@ -17398,15 +17416,20 @@ rs6000_is_valid_shift_mask (rtx mask, rt
const char *
rs6000_insn_for_shift_mask (machine_mode mode, rtx *operands, bool dot)
{
- int nb, ne;
+ int nb, ne, n, sh;
+ rtx_code code;
- if (!rs6000_is_valid_mask (operands[3], &nb, &ne, mode))
+ if (!rs6000_is_valid_shift_mask_1 (operands[3], operands[4], mode, &nb, &ne,
+ &n, &sh, &code))
gcc_unreachable ();
+ if (sh != -1)
+ operands[2] = GEN_INT (sh);
+
if (mode == DImode && ne == 0)
{
- if (GET_CODE (operands[4]) == LSHIFTRT && INTVAL (operands[2]))
- operands[2] = GEN_INT (64 - INTVAL (operands[2]));
+ if (code == LSHIFTRT && sh)
+ operands[2] = GEN_INT (64 - sh);
operands[3] = GEN_INT (63 - nb);
if (dot)
return "rld%I2cl. %0,%1,%2,%3";
@@ -17422,9 +17445,8 @@ rs6000_insn_for_shift_mask (machine_mode
}
if (mode == DImode
- && GET_CODE (operands[4]) != LSHIFTRT
- && CONST_INT_P (operands[2])
- && ne == INTVAL (operands[2]))
+ && code != LSHIFTRT
+ && ne == sh)
{
operands[3] = GEN_INT (63 - nb);
if (dot)
@@ -17434,8 +17456,8 @@ rs6000_insn_for_shift_mask (machine_mode
if (nb < 32 && ne < 32)
{
- if (GET_CODE (operands[4]) == LSHIFTRT && INTVAL (operands[2]))
- operands[2] = GEN_INT (32 - INTVAL (operands[2]));
+ if (code == LSHIFTRT && sh)
+ operands[2] = GEN_INT (32 - sh);
operands[3] = GEN_INT (31 - nb);
operands[4] = GEN_INT (31 - ne);
if (dot)
@@ -17453,32 +17475,14 @@ rs6000_insn_for_shift_mask (machine_mode
bool
rs6000_is_valid_insert_mask (rtx mask, rtx shift, machine_mode mode)
{
- int nb, ne;
-
- if (!rs6000_is_valid_mask (mask, &nb, &ne, mode))
- return false;
-
- int n = GET_MODE_PRECISION (mode);
+ int nb, ne, n, sh;
+ rtx_code code;
- int sh = INTVAL (XEXP (shift, 1));
- if (sh < 0 || sh >= n)
+ if (!rs6000_is_valid_shift_mask_1 (mask, shift, mode, &nb, &ne,
+ &n, &sh, &code)
+ || sh < 0)
return false;
- rtx_code code = GET_CODE (shift);
-
- /* Convert any shift by 0 to a rotate, to simplify below code. */
- if (sh == 0)
- code = ROTATE;
-
- /* Convert rotate to simple shift if we can, to make analysis simpler. */
- 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;
- }
-
/* DImode rotates need rldimi. */
if (mode == DImode && code == ROTATE)
return (ne == sh);
@@ -17517,17 +17521,21 @@ rs6000_is_valid_insert_mask (rtx mask, r
const char *
rs6000_insn_for_insert_mask (machine_mode mode, rtx *operands, bool dot)
{
- int nb, ne;
+ int nb, ne, n, sh;
+ rtx_code code;
- if (!rs6000_is_valid_mask (operands[3], &nb, &ne, mode))
+ if (!rs6000_is_valid_shift_mask_1 (operands[3], operands[4], mode, &nb, &ne,
+ &n, &sh, &code)
+ || sh < 0)
gcc_unreachable ();
/* Prefer rldimi because rlwimi is cracked. */
if (TARGET_POWERPC64
&& (!dot || mode == DImode)
- && GET_CODE (operands[4]) != LSHIFTRT
- && ne == INTVAL (operands[2]))
+ && code != LSHIFTRT
+ && ne == sh)
{
+ operands[2] = GEN_INT (sh);
operands[3] = GEN_INT (63 - nb);
if (dot)
return "rldimi. %0,%1,%2,%3";
@@ -17536,8 +17544,10 @@ rs6000_insn_for_insert_mask (machine_mod
if (nb < 32 && ne < 32)
{
- if (GET_CODE (operands[4]) == LSHIFTRT && INTVAL (operands[2]))
- operands[2] = GEN_INT (32 - INTVAL (operands[2]));
+ if (code == LSHIFTRT && sh)
+ operands[2] = GEN_INT (32 - sh);
+ else
+ operands[2] = GEN_INT (sh);
operands[3] = GEN_INT (31 - nb);
operands[4] = GEN_INT (31 - ne);
if (dot)
--- gcc/testsuite/gcc.dg/pr69946.c.jj 2016-02-26 12:12:09.576530943 +0100
+++ gcc/testsuite/gcc.dg/pr69946.c 2016-02-26 12:11:50.000000000 +0100
@@ -0,0 +1,31 @@
+/* PR target/69946 */
+/* { dg-do assemble } */
+/* { dg-options "-O2" } */
+
+struct A
+{
+ int a : 4;
+ int : 2;
+ int b : 2;
+ int : 2;
+ int c : 2;
+ int d : 1;
+ int e;
+};
+struct B
+{
+ int a : 4;
+} *a;
+void bar (struct A);
+
+void
+foo (void)
+{
+ struct B b = a[0];
+ struct A c;
+ c.a = b.a;
+ c.b = 1;
+ c.c = 1;
+ c.d = 0;
+ bar (c);
+}
Jakub
