This is the mail archive of the
mailing list for the GCC project.
Re: [RFA:] Fix PR target/18701, address adjustment error withparadoxical mem subregs. (Disallow them, I say!)
- From: Roger Sayle <roger at eyesopen dot com>
- To: Hans-Peter Nilsson <hp at bitrange dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Thu, 30 Dec 2004 07:51:43 -0700 (MST)
- Subject: Re: [RFA:] Fix PR target/18701, address adjustment error withparadoxical mem subregs. (Disallow them, I say!)
On Wed, 29 Dec 2004, Hans-Peter Nilsson wrote:
> > Let me give your first patch the attention it deserves.
> I'll try to remember to adjust for turkey at this time of the
> year. ;-)
Ah, I've now analysed through the "tryptophan haze". The bit that I
found confusing was your e-mail contained two examples of unsafe
recombinations followed by two patches. The second example corresponded
to your second patch, with which I agreed and originally approved.
The hangup was that I didn't/don't think the first transformation you
gave is incorrect. Fortunately, that doesn't matter as your first
patch has nothing to do with your first example!
> PR target/18701
> * combine.c (gen_lowpart_for_combine): Re-read the mode of
> possibly-subreg-stripped x before doing offset calculations for
> address adjustment.
This is also OK for mainline, but with one minor tweak. Could you
move your change to correctly update imode and isize from the MEM_P
clause you propose, into the "GET_CODE (x) === SUBREG" clause before
the call to gen_lowpart_common? Conceptually, its when we change
the value of "x" that we need to update it's mode (imode) and it's
< if (GET_CODE (x) == SUBREG && MEM_P (SUBREG_REG (x)))
< x = SUBREG_REG (x);
< if (GET_MODE (x) == omode)
< return x;
> if (GET_CODE (x) == SUBREG && MEM_P (SUBREG_REG (x)))
> x = SUBREG_REG (x);
> imode = GET_MODE (x);
> if (imode == omode)
> return x;
> isize = GET_MODE_SIZE (imode);
but with a suitable comment and ChangeLog entry, blah, blah, blah...
If the above revisions successfully bootstrap and regression test for
you and continue to resolve PR target/18701, consider it preapproved.
We can now resume our scheduled snowball fight on why the middle-end
can't eliminate paradoxical subregs of MEMs until target maintainers
upgrade from LOAD_EXTEND_OP to using explicit zero_extend and/or
sign_extend in their machine descriptions :>