The attached patch takes care of the problem described below.
(I tried to stick the new code into the regular combine-simplify
stuff, but that does not work well. At the time when
combine_simplify_rtx is called, it's not known whether
change_zero_ext will expand the ZERO_EXTRACT to "(and (lshiftrt"
and does not know the constant it could pull out of the zero
extract, so this is why the patch does it in change_zero_ext().)
To take the code wrapping the ZERO_EXTRACT into account it was
necessary to move the code dealing with the set source to a
separate function that is called recursively.
The patch has been bootstrapped and regression tested on s390 and
s390x and has no noticeably bad side effects so far.
P.S.: Is it necessary to pass PITER by pointer or could it be
passed simply by value?
On Wed, Dec 07, 2016 at 09:21:35PM +0100, Dominik Vogt wrote:
> I guess this has something to do with the extra logic for
> replacing zero_extract with "(and (lshiftrt ...". In one specific
> situation, the replacement generates something odd. Initially,
> there are these insns:
>
> --
> (insn 6 3 7 2 (set (reg:DI 67)
> (lshiftrt:DI (reg:DI 2 %r2 [ v_x ])
> (const_int 12 [0xc])))
>
> (insn 7 6 8 2 (parallel [
> (set (reg:SI 66 [ v_conv ])
> (and:SI (subreg:SI (reg:DI 67) 4)
> (const_int -4 [0xfffffffffffffffc])))
> (clobber (reg:CC 33 %cc))
> --
>
> Combine tries:
>
> --
> Trying 6 -> 7:
> Failed to match this instruction:
> (parallel [
> (set (reg:SI 66 [ v_conv ])
> (and:SI (subreg:SI (zero_extract:DI (reg:DI 2 %r2 [
> v_x ])
> (const_int 32 [0x20])
> (const_int 20 [0x14])) 4)
> (const_int -4 [0xfffffffffffffffc])))
> (clobber (reg:CC 33 %cc))
> ])
> ...
> (const_int -4 [0xfffffffffffffffc])))
> Failed to match this instruction:
> (set (reg:SI 66 [ v_conv ])
> (and:SI (subreg:SI (and:DI (lshiftrt:DI (reg:DI 2 %r2 [ v_x ])
> ^^^^^^^^^^^^^^^^^^^^^^^^^^
> (const_int 12 [0xc]))
> (const_int 4294967295 [0xffffffff])) 4)
> (const_int -4 [0xfffffffffffffffc])))
>
> --
>
> The two "and"s could be combined into a single one. Now, I can of
> course add some pattern to the backend to deal with it, but this
> should probably be handled in combin?
>
> (The C code is (with the proper types):)
> --
> #define i64 signed long long
> #define ui64 unsigned long long
> #define i32 signed int
> #define ui32 unsigned int
> i32 f43 (i64 v_x)
> {
> i64 v_shr3 = ((ui64)v_x) >> 12;
> i32 v_shr3_tr = (ui32)v_shr3;
> i32 v_conv = v_shr3_tr & -4;
> return v_conv;
> }
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
gcc/ChangeLog-change_zero_ext-4
* combine.c (change_zero_ext, change_zero_ext_src): Move code dealing
with the source to new function.
Replace "(and (subreg (and ..." with "(and (subreg (...".
>From d6926e6ae23ea056223e5053220c398fa2b06f0d Mon Sep 17 00:00:00 2001
From: Dominik Vogt <v...@linux.vnet.ibm.com>
Date: Fri, 9 Dec 2016 19:13:45 +0100
Subject: [PATCH] combine: Simplify "(and (subreg (zero_extract" in
change_zero_ext.
When change_zero_ext expands
(and:SI (subreg:SI (zero_extract:DI ...
it ends up with two nested ANDs:
(and:SI (subreg:SI (and:DI (lshiftrt:DI ...
Which requires special support from the backend. So, merge the inner AND
constant into the outer and dump the inner AND.
At the time make_compound_statement is called, the code does not know that the
zero_extract may be expanded to "(and (lshiftrt" later (in change_zero_ext) and
should not make that simplification itself.
---
gcc/combine.c | 172 +++++++++++++++++++++++++++++++++++++---------------------
1 file changed, 109 insertions(+), 63 deletions(-)
diff --git a/gcc/combine.c b/gcc/combine.c
index 9c7999b..afa20a7 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -11216,81 +11216,127 @@ recog_for_combine_1 (rtx *pnewpat, rtx_insn *insn,
rtx *pnotes)
return insn_code_number;
}
-/* Change every ZERO_EXTRACT and ZERO_EXTEND of a SUBREG that can be
- expressed as an AND and maybe an LSHIFTRT, to that formulation.
- Return whether anything was so changed. */
-
+/* Helper function for change_zero_ext that processes a single
+ subexpression of the SET source passed in through PITER. */
static bool
-change_zero_ext (rtx pat)
+change_zero_ext_src (subrtx_ptr_iterator *piter)
{
bool changed = false;
- rtx *src = &SET_SRC (pat);
+ rtx x = ***piter;
+ machine_mode mode = GET_MODE (x);
+ int size;
+
+ if (GET_CODE (x) == AND
+ && CONST_INT_P (XEXP (x, 1))
+ && GET_CODE (XEXP (x, 0)) == SUBREG
+ && subreg_lowpart_p (XEXP (x, 0))
+ && GET_CODE (XEXP (XEXP (x, 0), 0)) == ZERO_EXTRACT
+ && GET_MODE_PRECISION (mode)
+ < GET_MODE_PRECISION (GET_MODE (XEXP (XEXP (x, 0), 0))))
+ {
+ subrtx_ptr_iterator::array_type array;
+ subrtx_ptr_iterator tmp_iter (array, &XEXP (XEXP (x, 0), 0),
+ NONCONST_BOUNDS);
+
+ change_zero_ext_src (&tmp_iter);
+ rtx sr = XEXP (x, 0);
+ rtx inner = XEXP (sr, 0);
+ if (GET_CODE (inner) == AND
+ && CONST_INT_P (XEXP (inner, 1)))
+ {
+ machine_mode inner_mode = GET_MODE (inner);
+
+ /* Merge the inner AND into the outer. */
+ int pmax
+ = MAX (GET_MODE_PRECISION (mode), GET_MODE_PRECISION (inner_mode));
+ int pmin
+ = MIN (GET_MODE_PRECISION (mode), GET_MODE_PRECISION (inner_mode));
+
+ wide_int mask = wi::mask (pmin, false, pmax);
+ mask &= wi::uhwi (UINTVAL (XEXP (x, 1)), pmax);
+ mask &= wi::uhwi (UINTVAL (XEXP (inner, 1)), pmax);
+ rtx y = gen_rtx_SUBREG (mode, XEXP (inner, 0), SUBREG_BYTE (sr));
+ x = gen_rtx_AND (mode, y, immed_wide_int_const (mask, mode));
+ SUBST (***piter, x);
+ return true;
+ }
+ else
+ return false;
+ }
- subrtx_ptr_iterator::array_type array;
- FOR_EACH_SUBRTX_PTR (iter, array, src, NONCONST)
+ if (GET_CODE (x) == ZERO_EXTRACT
+ && CONST_INT_P (XEXP (x, 1))
+ && CONST_INT_P (XEXP (x, 2))
+ && GET_MODE (XEXP (x, 0)) != VOIDmode
+ && GET_MODE_PRECISION (GET_MODE (XEXP (x, 0)))
+ <= GET_MODE_PRECISION (mode))
{
- rtx x = **iter;
- machine_mode mode = GET_MODE (x);
- int size;
-
- if (GET_CODE (x) == ZERO_EXTRACT
- && CONST_INT_P (XEXP (x, 1))
- && CONST_INT_P (XEXP (x, 2))
- && GET_MODE (XEXP (x, 0)) != VOIDmode
- && GET_MODE_PRECISION (GET_MODE (XEXP (x, 0)))
- <= GET_MODE_PRECISION (mode))
- {
- machine_mode inner_mode = GET_MODE (XEXP (x, 0));
+ machine_mode inner_mode = GET_MODE (XEXP (x, 0));
- size = INTVAL (XEXP (x, 1));
+ size = INTVAL (XEXP (x, 1));
- int start = INTVAL (XEXP (x, 2));
- if (BITS_BIG_ENDIAN)
- start = GET_MODE_PRECISION (inner_mode) - size - start;
+ int start = INTVAL (XEXP (x, 2));
+ if (BITS_BIG_ENDIAN)
+ start = GET_MODE_PRECISION (inner_mode) - size - start;
- if (start)
- x = gen_rtx_LSHIFTRT (inner_mode, XEXP (x, 0), GEN_INT (start));
- else
- x = XEXP (x, 0);
- if (mode != inner_mode)
- x = gen_lowpart_SUBREG (mode, x);
- }
- else if (GET_CODE (x) == ZERO_EXTEND
- && SCALAR_INT_MODE_P (mode)
- && GET_CODE (XEXP (x, 0)) == SUBREG
- && SCALAR_INT_MODE_P (GET_MODE (SUBREG_REG (XEXP (x, 0))))
- && !paradoxical_subreg_p (XEXP (x, 0))
- && subreg_lowpart_p (XEXP (x, 0)))
- {
- size = GET_MODE_PRECISION (GET_MODE (XEXP (x, 0)));
- x = SUBREG_REG (XEXP (x, 0));
- if (GET_MODE (x) != mode)
- x = gen_lowpart_SUBREG (mode, x);
- }
- else if (GET_CODE (x) == ZERO_EXTEND
- && SCALAR_INT_MODE_P (mode)
- && REG_P (XEXP (x, 0))
- && HARD_REGISTER_P (XEXP (x, 0))
- && can_change_dest_mode (XEXP (x, 0), 0, mode))
- {
- size = GET_MODE_PRECISION (GET_MODE (XEXP (x, 0)));
- x = gen_rtx_REG (mode, REGNO (XEXP (x, 0)));
- }
+ if (start)
+ x = gen_rtx_LSHIFTRT (inner_mode, XEXP (x, 0), GEN_INT (start));
else
- continue;
-
- if (!(GET_CODE (x) == LSHIFTRT
- && CONST_INT_P (XEXP (x, 1))
- && size + INTVAL (XEXP (x, 1)) == GET_MODE_PRECISION (mode)))
- {
- wide_int mask = wi::mask (size, false, GET_MODE_PRECISION (mode));
- x = gen_rtx_AND (mode, x, immed_wide_int_const (mask, mode));
- }
+ x = XEXP (x, 0);
+ if (mode != inner_mode)
+ x = gen_lowpart_SUBREG (mode, x);
+ }
+ else if (GET_CODE (x) == ZERO_EXTEND
+ && SCALAR_INT_MODE_P (mode)
+ && GET_CODE (XEXP (x, 0)) == SUBREG
+ && SCALAR_INT_MODE_P (GET_MODE (SUBREG_REG (XEXP (x, 0))))
+ && !paradoxical_subreg_p (XEXP (x, 0))
+ && subreg_lowpart_p (XEXP (x, 0)))
+ {
+ size = GET_MODE_PRECISION (GET_MODE (XEXP (x, 0)));
+ x = SUBREG_REG (XEXP (x, 0));
+ if (GET_MODE (x) != mode)
+ x = gen_lowpart_SUBREG (mode, x);
+ }
+ else if (GET_CODE (x) == ZERO_EXTEND
+ && SCALAR_INT_MODE_P (mode)
+ && REG_P (XEXP (x, 0))
+ && HARD_REGISTER_P (XEXP (x, 0))
+ && can_change_dest_mode (XEXP (x, 0), 0, mode))
+ {
+ size = GET_MODE_PRECISION (GET_MODE (XEXP (x, 0)));
+ x = gen_rtx_REG (mode, REGNO (XEXP (x, 0)));
+ }
+ else
+ return changed;
- SUBST (**iter, x);
- changed = true;
+ if (!(GET_CODE (x) == LSHIFTRT
+ && CONST_INT_P (XEXP (x, 1))
+ && size + INTVAL (XEXP (x, 1)) == GET_MODE_PRECISION (mode)))
+ {
+ wide_int mask = wi::mask (size, false, GET_MODE_PRECISION (mode));
+ x = gen_rtx_AND (mode, x, immed_wide_int_const (mask, mode));
}
+ SUBST (***piter, x);
+ return true;
+}
+
+/* Change every ZERO_EXTRACT and ZERO_EXTEND of a SUBREG that can be
+ expressed as an AND and maybe an LSHIFTRT, to that formulation.
+ Peel of SIGN_EXTEND from expressions containing a ZERO_EXTRACT if the
+ SIGN_EXTEND is a no-op. Return whether anything was so changed. */
+
+static bool
+change_zero_ext (rtx pat)
+{
+ bool changed = false;
+ rtx *src = &SET_SRC (pat);
+
+ subrtx_ptr_iterator::array_type array;
+ FOR_EACH_SUBRTX_PTR (iter, array, src, NONCONST)
+ changed |= change_zero_ext_src (&iter);
+
if (changed)
FOR_EACH_SUBRTX_PTR (iter, array, src, NONCONST)
maybe_swap_commutative_operands (**iter);
--
2.3.0
