"H.J. Lu" <hjl.to...@gmail.com> writes:
> diff --git a/gcc/combine.c b/gcc/combine.c
> index a07c046..b9a3589 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -10784,12 +10784,30 @@ gen_lowpart_for_combine (enum machine_mode omode,
> rtx
> x)
> if (omode == imode)
> return x;
>
> - /* Return identity if this is a CONST or symbolic reference. */
> - if (omode == Pmode
> - && (GET_CODE (x) == CONST
> - || GET_CODE (x) == SYMBOL_REF
> - || GET_CODE (x) == LABEL_REF))
> - return x;
> + if (omode == Pmode)
> + {
> + /* Return identity if this is a symbolic reference. */
> + if (GET_CODE (x) == SYMBOL_REF || GET_CODE (x) == LABEL_REF)
> + return x;
> +
> + /* Return identity for CONST unless this is a PLUS of 2 constant
> + operands. */
> + if (GET_CODE (x) == CONST)
> + {
> + rtx y = XEXP (x, 0);
> + if (GET_CODE (y) == PLUS
> + && ((CONST_INT_P (XEXP (y, 1))
> + && (GET_CODE (XEXP (y, 0)) == CONST
> + || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
> + || GET_CODE (XEXP (y, 0)) == LABEL_REF))
> + || (CONST_INT_P (XEXP (y, 1))
> + && (GET_CODE (XEXP (y, 0)) == CONST
> + || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
> + || GET_CODE (XEXP (y, 0)) == LABEL_REF))))
> + goto fail;
> + return x;
> + }
> + }
>
> /* We can only support MODE being wider than a word if X is a
> constant integer or has a mode the same size. */
>
> works for the testcase.
My point was that the "return x" is always wrong. Whenever we return x
here, we know we're returning something in a different mode from the one
that the caller wanted. Returning a Pmode LABEL_REF might not trigger
that plus_constant assert, but it's still wrong.
It looks like this came from the mips-rewrite branch:
2003-03-13 Richard Sandiford <rsand...@redhat.com>
* combine.c (gen_lowpart_for_combine): Treat the lowpart Pmode of
a CONST as identity. Check the return value of gen_lowpart_common.
so I can categorically confirm that the person who wrote it didn't
know what they were doing. It also means that this case was motivated
by an experiment to make Pmode == DImode for n32, which we ended up
discarding because it produced worse code.
If this case really is important, we might consider using
convert_memory_address (Pmode, x) instead. I'm not sure whether
that would be right for targets with address spaces though, because
we don't know which address space is associated with the address.
Hopefully someone who knows address spaces can comment.
If it is correct, then it should probably go in gen_lowpart_common
rather than gen_lowpart_for_combine.
But given how few people have hit this, it doesn't look like a
particularly important attempted optimisation. I'll pre-approve
a patch that undoes my mistake and simply removes:
/* Return identity if this is a CONST or symbolic reference. */
if (omode == Pmode
&& (GET_CODE (x) == CONST
|| GET_CODE (x) == SYMBOL_REF
|| GET_CODE (x) == LABEL_REF))
return x;
Richard
