This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix gen_lowpart_if_possible (PR middle-end/85414)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Eric Botcazou <ebotcazou at adacore dot com>
- Cc: gcc-patches at gcc dot gnu dot org, Jeff Law <law at redhat dot com>
- Date: Tue, 17 Apr 2018 10:48:43 +0200
- Subject: Re: [PATCH] Fix gen_lowpart_if_possible (PR middle-end/85414)
- References: <20180416190157.GS8577@tucnak> <1641587.er2S7alkuO@polaris>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Tue, Apr 17, 2018 at 09:32:31AM +0200, Eric Botcazou wrote:
> > The following testcase FAILs, because cse_local sees
> > (zero_extend:TI (subreg/s/v:DI (reg:TI ...) 0))
> > inside of REG_EQUAL note, and simplify-rtx.c attempts to optimize it.
> > case ZERO_EXTEND:
> > /* Check for a zero extension of a subreg of a promoted
> > variable, where the promotion is zero-extended, and the
> > target mode is the same as the variable's promotion. */
> > if (GET_CODE (op) == SUBREG
> > && SUBREG_PROMOTED_VAR_P (op)
> > && SUBREG_PROMOTED_UNSIGNED_P (op)
> > && !paradoxical_subreg_p (mode, GET_MODE (SUBREG_REG (op))))
> > {
> > temp = rtl_hooks.gen_lowpart_no_emit (mode, op);
> > if (temp)
> > return temp;
> > }
>
> This code is strange though, here's the equivalent one in convert_modes:
>
> if (GET_CODE (x) == SUBREG
> && SUBREG_PROMOTED_VAR_P (x)
> && is_a <scalar_int_mode> (mode, &int_mode)
> && (GET_MODE_PRECISION (subreg_promoted_mode (x))
> >= GET_MODE_PRECISION (int_mode))
> && SUBREG_CHECK_PROMOTED_SIGN (x, unsignedp))
> x = gen_lowpart (int_mode, SUBREG_REG (x));
Yes, convert_modes does this, on the other side e.g. convert_move a few
hundred lines above it doesn't:
if (GET_CODE (from) == SUBREG
&& SUBREG_PROMOTED_VAR_P (from)
&& is_a <scalar_int_mode> (to_mode, &to_int_mode)
&& (GET_MODE_PRECISION (subreg_promoted_mode (from))
>= GET_MODE_PRECISION (to_int_mode))
&& SUBREG_CHECK_PROMOTED_SIGN (from, unsignedp))
from = gen_lowpart (to_int_mode, from), from_mode = to_int_mode;
> so can't we just pass SUBREG_REG (op) here? Same for SIGN_EXTEND above.
The following patch works for the testcase too (even with the rtlhooks.c
hunk removed, but I think we want to keep it anyway, even when we lose the
only known testcase where it matters). We generate the same assembly,
though it behaves differently. With the previous patch we have:
(zero_extend:TI (subreg/s/v:DI (reg:TI 94) 0))
in the REG_EQUAL note, then subreg2 makes (zero_extend:TI (reg:DI 146))
out of it and in the end nothing uses the REG_EQUAL note.
With this patch, we have instead (reg:TI 94) in the REG_EQUAL note and
subreg2 pass drops the REG_EQUAL note, so nothing can use it even if it
wanted.
Anyway, if this is what is preferred, ok for trunk if it passes
bootstrap/regtest?
2018-04-17 Jakub Jelinek <jakub@redhat.com>
PR middle-end/85414
* simplify-rtx.c (simplify_unary_operation_1) <case SIGN_EXTEND,
case ZERO_EXTEND>: Pass SUBREG_REG (op) rather than op to
gen_lowpart_no_emit.
* rtlhooks.c (gen_lowpart_if_possible): Don't call gen_lowpart_SUBREG
on a SUBREG.
* gcc.dg/pr85414.c: New test.
--- gcc/simplify-rtx.c.jj 2018-04-13 21:38:39.924432794 +0200
+++ gcc/simplify-rtx.c 2018-04-17 10:37:57.559193098 +0200
@@ -1484,7 +1484,7 @@ simplify_unary_operation_1 (enum rtx_cod
&& SUBREG_PROMOTED_SIGNED_P (op)
&& !paradoxical_subreg_p (mode, GET_MODE (SUBREG_REG (op))))
{
- temp = rtl_hooks.gen_lowpart_no_emit (mode, op);
+ temp = rtl_hooks.gen_lowpart_no_emit (mode, SUBREG_REG (op));
if (temp)
return temp;
}
@@ -1567,7 +1567,7 @@ simplify_unary_operation_1 (enum rtx_cod
&& SUBREG_PROMOTED_UNSIGNED_P (op)
&& !paradoxical_subreg_p (mode, GET_MODE (SUBREG_REG (op))))
{
- temp = rtl_hooks.gen_lowpart_no_emit (mode, op);
+ temp = rtl_hooks.gen_lowpart_no_emit (mode, SUBREG_REG (op));
if (temp)
return temp;
}
--- gcc/rtlhooks.c.jj 2018-04-16 18:11:54.791378162 +0200
+++ gcc/rtlhooks.c 2018-04-17 10:39:43.263246431 +0200
@@ -123,9 +123,9 @@ gen_lowpart_if_possible (machine_mode mo
return new_rtx;
}
- else if (mode != GET_MODE (x) && GET_MODE (x) != VOIDmode
+ else if (mode != GET_MODE (x) && GET_MODE (x) != VOIDmode && !SUBREG_P (x)
&& validate_subreg (mode, GET_MODE (x), x,
- subreg_lowpart_offset (mode, GET_MODE (x))))
+ subreg_lowpart_offset (mode, GET_MODE (x))))
return gen_lowpart_SUBREG (mode, x);
else
return 0;
--- gcc/testsuite/gcc.dg/pr85414.c.jj 2018-04-17 10:21:55.531391008 +0200
+++ gcc/testsuite/gcc.dg/pr85414.c 2018-04-17 10:21:55.531391008 +0200
@@ -0,0 +1,10 @@
+/* PR middle-end/85414 */
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-Og -fgcse -Wno-uninitialized" } */
+
+int
+foo (void)
+{
+ unsigned __int128 c;
+ return __builtin_mul_overflow_p (59, -c, (short) 0);
+}
Jakub