RFA: Fix DF--subreg->DI--truncate->SI problems in combine.c
Richard Sandiford
rdsandiford@googlemail.com
Wed Aug 5 18:43:00 GMT 2009
This patch restores mipsisa64-elf builds. The problem is that we
try to force the expression:
(and:DI (subreg:DI (reg:DF foo) 0)
(const_int 1))
into SImode. Let the whole expression be A, the subreg be S and
the reg be R. force_to_mode simplifies the expression by calling:
simplify_and_const_int (A, SImode, S, 1)
which looks odd, but is OK: simplify_and_const allows the operands
to have a different mode from the result. It handles this by calling:
force_to_mode (SImode, S, ...)
and the subreg case of that function calls:
force_to_mode (SImode, R, ...)
At this point we're truncating a DFmode register into SImode.
This results in a call:
gen_lowpart_or_truncate (SImode, R)
which produces:
(truncate:SI (reg:DF foo))
And that's not a meaningful combination of modes. The question then is:
where did things go wrong? The comments above force_to_mode don't
really say what the preconditions are, but I think it's obvious from
extracts like:
op_mode = ((GET_MODE_CLASS (mode) == GET_MODE_CLASS (GET_MODE (x))
&& have_insn_for (code, mode))
? mode : GET_MODE (x));
and:
if (VECTOR_MODE_P (mode) || VECTOR_MODE_P (GET_MODE (x)))
return gen_lowpart (mode, x);
that the function is being used for arbitrary modes. (Experimenting with
a few asserts bears this out.) This means that gen_lowpart_or_truncate
should also handle arbitrary modes. Furthermore, although the SUBREG
implied by:
gen_lowpart_or_truncate (SImode, R)
is not valid, it isn't the caller's responsibility to check this.
Instead, gen_lowpart_for_combine returns a (clobber:SI (const_int 0)).
So I think the truncate code in gen_lowpart_of_truncate should
be prepared to handle the general (possibly invalid) case too.
(And TRULY_NOOP_TRUNCATION _should_ be applied to non-integer modes,
since we would otherwise lose the truncation if we did conversions
such as V2SF -> SF, then -- perhaps separately -- SF -> SI.)
Another thing bugged me while looking at this. The handling of
binary operations is:
/* For most binary operations, just propagate into the operation and
change the mode if we have an operation of that mode. */
op0 = force_to_mode (XEXP (x, 0), mode, mask, next_select);
op1 = force_to_mode (XEXP (x, 1), mode, mask, next_select);
...
op0 = gen_lowpart_or_truncate (op_mode, op0);
op1 = gen_lowpart_or_truncate (op_mode, op1);
if (op_mode != GET_MODE (x) || op0 != XEXP (x, 0) || op1 != XEXP (x, 1))
x = simplify_gen_binary (code, op_mode, op0, op1);
I think it's clear that this only makes sense for integer modes,
but the vector condition quoted above shows that we're not just
getting integer modes. So I think it would be safer to use an
inclusive SCALAR_INT_MODE check than an exclusive !VECTOR_MODE_P check.
Bootstrapped & regression-tested on x86_64-linux-gnu. Also tested
on mipsisa64-elf, where the build is restored. OK to install?
Richard
gcc/
* combine.c (gen_lowpart_or_truncate): Exclude CONST_INTs from
mode check. Do truncations in an integer mode.
(force_to_mode): Handle subregs for all mode types. Only do
arithmetic simplifications on integer modes.
Index: gcc/combine.c
===================================================================
--- gcc/combine.c 2009-08-05 18:43:42.000000000 +0100
+++ gcc/combine.c 2009-08-05 18:44:51.000000000 +0100
@@ -7245,13 +7245,20 @@ canon_reg_for_combine (rtx x, rtx reg)
static rtx
gen_lowpart_or_truncate (enum machine_mode mode, rtx x)
{
- if (GET_MODE_SIZE (GET_MODE (x)) <= GET_MODE_SIZE (mode)
- || TRULY_NOOP_TRUNCATION (GET_MODE_BITSIZE (mode),
- GET_MODE_BITSIZE (GET_MODE (x)))
- || (REG_P (x) && reg_truncated_to_mode (mode, x)))
- return gen_lowpart (mode, x);
- else
- return simplify_gen_unary (TRUNCATE, mode, x, GET_MODE (x));
+ if (!CONST_INT_P (x)
+ && GET_MODE_SIZE (mode) < GET_MODE_SIZE (GET_MODE (x))
+ && !TRULY_NOOP_TRUNCATION (GET_MODE_BITSIZE (mode),
+ GET_MODE_BITSIZE (GET_MODE (x)))
+ && !(REG_P (x) && reg_truncated_to_mode (mode, x)))
+ {
+ /* Bit-cast X into an integer mode. */
+ if (!SCALAR_INT_MODE_P (GET_MODE (x)))
+ x = gen_lowpart (int_mode_for_mode (GET_MODE (x)), x);
+ x = simplify_gen_unary (TRUNCATE, int_mode_for_mode (mode),
+ x, GET_MODE (x));
+ }
+
+ return gen_lowpart (mode, x);
}
/* See if X can be simplified knowing that we will only refer to it in
@@ -7338,9 +7345,20 @@ force_to_mode (rtx x, enum machine_mode
&& (GET_MODE_MASK (GET_MODE (x)) & ~mask) == 0)
return gen_lowpart (mode, x);
- /* The arithmetic simplifications here do the wrong thing on vector modes. */
- if (VECTOR_MODE_P (mode) || VECTOR_MODE_P (GET_MODE (x)))
- return gen_lowpart (mode, x);
+ /* We can ignore the effect of a SUBREG if it narrows the mode or
+ if the constant masks to zero all the bits the mode doesn't have. */
+ if (GET_CODE (x) == SUBREG
+ && subreg_lowpart_p (x)
+ && ((GET_MODE_SIZE (GET_MODE (x))
+ < GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))))
+ || (0 == (mask
+ & GET_MODE_MASK (GET_MODE (x))
+ & ~GET_MODE_MASK (GET_MODE (SUBREG_REG (x)))))))
+ return force_to_mode (SUBREG_REG (x), mode, mask, next_select);
+
+ /* The arithmetic simplifications here only work for scalar integer modes. */
+ if (!SCALAR_INT_MODE_P (mode) || !SCALAR_INT_MODE_P (GET_MODE (x)))
+ return gen_lowpart_or_truncate (mode, x);
switch (code)
{
@@ -7358,19 +7376,6 @@ force_to_mode (rtx x, enum machine_mode
return force_to_mode (x, mode, mask, next_select);
break;
- case SUBREG:
- if (subreg_lowpart_p (x)
- /* We can ignore the effect of this SUBREG if it narrows the mode or
- if the constant masks to zero all the bits the mode doesn't
- have. */
- && ((GET_MODE_SIZE (GET_MODE (x))
- < GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))))
- || (0 == (mask
- & GET_MODE_MASK (GET_MODE (x))
- & ~GET_MODE_MASK (GET_MODE (SUBREG_REG (x)))))))
- return force_to_mode (SUBREG_REG (x), mode, mask, next_select);
- break;
-
case TRUNCATE:
/* Similarly for a truncate. */
return force_to_mode (XEXP (x, 0), mode, mask, next_select);
More information about the Gcc-patches
mailing list