This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
pr 17531 finis
- From: Richard Henderson <rth at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Tue, 28 Sep 2004 16:00:40 -0700
- Subject: pr 17531 finis
I've committed *both* patches. Zdenek does have a point that later
on in expand_binop we do other mode conversion stuff.
I think the biggest problem shown by this PR is that it's a mistake
for expand_expr to not honor the mode in which it is asked to expand
things. It means that *every* user of expand_expr has to be prepared
to convert to the expected mode, and that any place that doesn't check
is buggy. Which is just insane.
I may see how much trouble is to be had by changing this for 4.1.
r~
PR 17531
* expr.c (expand_expr_addr_expr_1): Only assemble_external for decls.
Don't check VOIDmode here. Force PLUS operands to common type.
(expand_expr_addr_expr): Do VOIDmode check earlier. Force use of
Pmode if given a non pointer type.
PR 17531
* optabs.c (expand_binop): Force constants to the correct mode.
Index: expr.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.c,v
retrieving revision 1.724
diff -c -p -d -r1.724 expr.c
*** expr.c 28 Sep 2004 00:13:11 -0000 1.724
--- expr.c 28 Sep 2004 22:50:55 -0000
*************** expand_expr_addr_expr_1 (tree exp, rtx t
*** 6088,6094 ****
case CONST_DECL:
/* Recurse and make the output_constant_def clause above handle this. */
return expand_expr_addr_expr_1 (DECL_INITIAL (exp), target,
! tmode, modifier);
case REALPART_EXPR:
/* The real part of the complex number is always first, therefore
--- 6088,6094 ----
case CONST_DECL:
/* Recurse and make the output_constant_def clause above handle this. */
return expand_expr_addr_expr_1 (DECL_INITIAL (exp), target,
! tmode, modifier);
case REALPART_EXPR:
/* The real part of the complex number is always first, therefore
*************** expand_expr_addr_expr_1 (tree exp, rtx t
*** 6126,6132 ****
result = XEXP (result, 0);
/* ??? Is this needed anymore? */
! if (!TREE_USED (exp) == 0)
{
assemble_external (exp);
TREE_USED (exp) = 1;
--- 6126,6132 ----
result = XEXP (result, 0);
/* ??? Is this needed anymore? */
! if (DECL_P (exp) && !TREE_USED (exp) == 0)
{
assemble_external (exp);
TREE_USED (exp) = 1;
*************** expand_expr_addr_expr_1 (tree exp, rtx t
*** 6149,6161 ****
subtarget = offset || bitpos ? NULL_RTX : target;
result = expand_expr_addr_expr_1 (inner, subtarget, tmode, modifier);
- if (tmode == VOIDmode)
- {
- tmode = GET_MODE (result);
- if (tmode == VOIDmode)
- tmode = Pmode;
- }
-
if (offset)
{
rtx tmp;
--- 6149,6154 ----
*************** expand_expr_addr_expr_1 (tree exp, rtx t
*** 6164,6169 ****
--- 6157,6165 ----
result = force_operand (result, NULL);
tmp = expand_expr (offset, NULL, tmode, EXPAND_NORMAL);
+ result = convert_memory_address (tmode, result);
+ tmp = convert_memory_address (tmode, tmp);
+
if (modifier == EXPAND_SUM)
result = gen_rtx_PLUS (tmode, result, tmp);
else
*************** expand_expr_addr_expr_1 (tree exp, rtx t
*** 6178,6184 ****
{
/* Someone beforehand should have rejected taking the address
of such an object. */
! gcc_assert (!(bitpos % BITS_PER_UNIT));
result = plus_constant (result, bitpos / BITS_PER_UNIT);
if (modifier < EXPAND_SUM)
--- 6174,6180 ----
{
/* Someone beforehand should have rejected taking the address
of such an object. */
! gcc_assert ((bitpos % BITS_PER_UNIT) == 0);
result = plus_constant (result, bitpos / BITS_PER_UNIT);
if (modifier < EXPAND_SUM)
*************** expand_expr_addr_expr (tree exp, rtx tar
*** 6198,6216 ****
enum machine_mode rmode;
rtx result;
result = expand_expr_addr_expr_1 (TREE_OPERAND (exp, 0), target,
tmode, modifier);
/* Despite expand_expr claims concerning ignoring TMODE when not
! strictly convenient, stuff breaks if we don't honor it. */
! if (tmode == VOIDmode)
! tmode = TYPE_MODE (TREE_TYPE (exp));
rmode = GET_MODE (result);
if (rmode == VOIDmode)
rmode = tmode;
if (rmode != tmode)
result = convert_memory_address (tmode, result);
!
return result;
}
--- 6194,6221 ----
enum machine_mode rmode;
rtx result;
+ /* Target mode of VOIDmode says "whatever's natural". */
+ if (tmode == VOIDmode)
+ tmode = TYPE_MODE (TREE_TYPE (exp));
+
+ /* We can get called with some Weird Things if the user does silliness
+ like "(short) &a". In that case, convert_memory_address won't do
+ the right thing, so ignore the given target mode. */
+ if (!targetm.valid_pointer_mode (tmode))
+ tmode = Pmode;
+
result = expand_expr_addr_expr_1 (TREE_OPERAND (exp, 0), target,
tmode, modifier);
/* Despite expand_expr claims concerning ignoring TMODE when not
! strictly convenient, stuff breaks if we don't honor it. Note
! that combined with the above, we only do this for pointer modes. */
rmode = GET_MODE (result);
if (rmode == VOIDmode)
rmode = tmode;
if (rmode != tmode)
result = convert_memory_address (tmode, result);
!
return result;
}
Index: optabs.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/optabs.c,v
retrieving revision 1.245
diff -c -p -d -r1.245 optabs.c
*** optabs.c 28 Sep 2004 11:49:06 -0000 1.245
--- optabs.c 28 Sep 2004 22:50:55 -0000
*************** expand_binop (enum machine_mode mode, op
*** 834,844 ****
force expensive constants into a register. */
if (CONSTANT_P (op0) && optimize
&& rtx_cost (op0, binoptab->code) > COSTS_N_INSNS (1))
! op0 = force_reg (mode, op0);
if (CONSTANT_P (op1) && optimize
&& ! shift_op && rtx_cost (op1, binoptab->code) > COSTS_N_INSNS (1))
! op1 = force_reg (mode, op1);
/* Record where to delete back to if we backtrack. */
last = get_last_insn ();
--- 834,852 ----
force expensive constants into a register. */
if (CONSTANT_P (op0) && optimize
&& rtx_cost (op0, binoptab->code) > COSTS_N_INSNS (1))
! {
! if (GET_MODE (op0) != VOIDmode)
! op0 = convert_modes (mode, VOIDmode, op0, unsignedp);
! op0 = force_reg (mode, op0);
! }
if (CONSTANT_P (op1) && optimize
&& ! shift_op && rtx_cost (op1, binoptab->code) > COSTS_N_INSNS (1))
! {
! if (GET_MODE (op1) != VOIDmode)
! op1 = convert_modes (mode, VOIDmode, op1, unsignedp);
! op1 = force_reg (mode, op1);
! }
/* Record where to delete back to if we backtrack. */
last = get_last_insn ();