Hi!
On Wed, Sep 08, 2021 at 08:42:44AM +0200, Richard Biener wrote:
> On Wed, Sep 8, 2021 at 1:08 AM Segher Boessenkool
> <seg...@kernel.crashing.org> wrote:
> > The core of the problem is that subreg of pseudos has three meanings:
> > -- Paradoxical subregs;
> > -- Actual subregs;
> > -- "bit_cast" thingies: treat the same bits as something else. Like
> > looking at the bits of a float as its memory image.
> >
> > Ignoring paradoxical subregs (as well as subregs of mem, which should
> > have disappeared by now), and subregs of hard registers as well (those
> > have *different* semantics after all), the other two kinds can be mixed,
> > and *have to* be mixed, because subregs of subregs are non-canonical.
> >
> > Is there any reason why not to allow this kind of subreg?
>
> In fact the causing rev. in question
> (d2874d905647a1d146dafa60199d440e837adc4d)
> made all those subregs "valid" in terms of what validate_subreg is verifying
> and thus now the few places using validate_subreg to check whether some
> subreg is valid will now happily do float<->int converting subregs.
Like in PR102154:
error: unrecognizable insn:
(insn 11 10 12 2 (set (reg:SF 118 [ <retval> ])
(subreg:SF (reg:DI 122) 0))
This generated valid code before, and now it is not anymore. If the
patch was meant to allow *more*, it failed that goal: it allows *less*.
The issue is that we do not allow SF subregs in all cases. We do not
store SFmode quantities in IEEE SP format in registers normally, so it
would require expensive extra code to allow such subregs.
Whatever pass made these subregs is wrong, because they do not pass
recog().
> I do agree that those subregs should be allowed and that the above rev. is
> a strict improvement (given it removes a lot of "but allow special case X
> because target Y wants it" cases by simply allowing all of them). But the
> previous code seems to have papered over quite some backend issues.
It is not a good idea to do allow all those things. Most backends can
only support a few combinations of them, and everything else results in
*worse* machine code, best case, and more and more complicated (and more
buggy!) backend code.
But that is a code quality issue. The current problem is that we have
at least PR102211 and PR102154 (as well as reports elsewhere of bugs on
other targets). Code that used before doesn't anymore, and we have no
clear way out, no recommendation how to fix this and a) keep the same
functionality without huge changes, and b) keep the same machine code
quality.
I do not think something like that can be done. That is why I am asking
for the patch to be reverted until all of the groundwork for it has been
done. That includes making generic testcases that show how such subregs
behave, so that we can see in testresults what changes do to existing
targets.
Segher
