Re: [RFA:] Fix PR target/18701, address adjustment error withparadoxical mem subregs. (Disallow them, I say!)

[Hans-Peter Nilsson <hp at bitrange dot com>]

On Thu, 30 Dec 2004, Roger Sayle wrote:
> 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!

(Where you write "example" I suggest it should be read
"transformation that happens for the test-case"; the
transformations aren't artificial examples.)

I originally had the patch order the other way round, i.e. in
order of creation, but consider the second patch slightly
inferior.  I guess that attempt to be turkey-friendly somewhat
misfired.

> >	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).

Will do.  (I did consider the patch to be along that line
originally, but thought it better to put the imode and isize
calculations where they were actually used.)

Thanks again for the review!

> 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  :>

I'll summarise (hopefully turkey-friendly: ;-) I don't think
LOAD_EXTEND_OP is conceptually related to SUBREGs: it's just
currently used *in some places* (maybe half of its uses) to
conveniently resolve the tie what to do with the "upper bits"
for paradoxical mem subregs.  That happens (and should happen
until paradoxical mem subregs are made extinct) both for targets
with as well as without explicit sign/zero_extend patterns.

Further, (forefinger in air), there's no need to create SUBREGs
to express sign and zero-extension in the absence of a
sign/zero_extend that matches an optimization.  Don't do that.
Just do what can be done with shifts or ands, and consider all
MEMs as extending when combine wants to do something clever with
a shift or and and a MEM, changing the SUBREG tests to plain MEM
tests.  Alternatively, at those times, create the actual
zero/sign_extend RTL and recog-match it to any MEM with a
suitable LOAD_EXTEND_OP.

brgds, H-P
PS. I suggest a change of subject-line with any follow-ups.