[RFA:] Fix PR target/18701, address adjustment error with paradoxical mem subregs. (Disallow them, I say!)
Roger Sayle
roger@eyesopen.com
Thu Dec 30 16:22:00 GMT 2004
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. ;-)
Hi H-P,
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
size (isize).
Something like:
< 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 :>
Thanks,
Roger
--
More information about the Gcc-patches
mailing list