Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Thu, Aug 26, 2021 at 11:06 AM Richard Sandiford
> <richard.sandif...@arm.com> wrote:
>>
>> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> > One thought I had is whether we can "fix" validate_subreg to have less
>> > "weird" allowed float-int
>> > special cases. As said upthread I think that we either should allow
>> > all of those, implying that
>> > subregs work semantically as if there's subregs to same-sized integer
>> > modes inbetween or
>> > disallow them all and make sure we're actually doing that explicitely.
>> >
>> > For example
>> >
>> > /* ??? Similarly, e.g. with (subreg:DF (reg:TI)). Though store_bit_field
>> > is the culprit here, and not the backends. */
>> > else if (known_ge (osize, regsize) && known_ge (isize, osize))
>> > ;
>> >
>> > I can't decipther rtl.text as to what the semantics of such a subreg is
>> > given the docs hand-wave about WORDS_BIG_ENDIAN vs.
>> > FLOAT_WORDS_BIG_ENDIAN but don't actually say what happens
>> > when you mix those in a subreg. So maybe the above should
>> > have explicitely have WORDS_BIG_ENDIAN == FLOAT_WORDS_BIG_ENDIAN.
>> >
>> > But then the world would be much simpler if subregs of non-same size
>> > modes have explicit documentation for the mode kinds we have.
>>
>> Yeah. Although validate_subreg was a good idea, some of the mode checks
>> are IMO a failed experiment. The hope was that eventually we'd remove
>> all those special exceptions once the culprit has been fixed. However,
>> the code is over 16 years old at this point and those changes never
>> happened.
>>
>> Nested subregs aren't a thing (thankfully) and one of the big disadvantages
>> of the current validate_subreg mode-changing rules is that they aren't
>> transitive. This can artificially require temporary pseudos for things
>> that could be expressed directly as a single subreg.
>
> And that's what the proposed patch does (add same-mode size integer mode
> punning intermediate subregs).
>
> So if that's not supposed to be necessary then why restrict subregs at all?
I was trying to say: I'm not sure we should.
> I mean you seem to imply that the semantics would be clear and well-defined
> (to you - not to me). The only thing is that of course not all subregs are
> "implemented" by a target (or can be, w/o spilling).
Yeah. That's for TARGET_CAN_CHANGE_MODE_CLASS to decide.
But it only comes in to play during RA or when trying to take
the subreg of a particular hard register. Transitivity doesn't
matter so much for the hard register case since the result of
simplify_gen_subreg should then be another hard register.
> Which means - we should adjust validate_subreg with another special-case
> or rather generalize the existing ones to an overall set that makes more
> sense?
Maybe it's too radical, but I would whether we should just get rid of:
/* ??? This should not be here. Temporarily continue to allow word_mode
subregs of anything. The most common offender is (subreg:SI (reg:DF)).
Generally, backends are doing something sketchy but it'll take time to
fix them all. */
if (omode == word_mode)
;
/* ??? Similarly, e.g. with (subreg:DF (reg:TI)). Though store_bit_field
is the culprit here, and not the backends. */
else if (known_ge (osize, regsize) && known_ge (isize, osize))
;
/* Allow component subregs of complex and vector. Though given the below
extraction rules, it's not always clear what that means. */
else if ((COMPLEX_MODE_P (imode) || VECTOR_MODE_P (imode))
&& GET_MODE_INNER (imode) == omode)
;
/* ??? x86 sse code makes heavy use of *paradoxical* vector subregs,
i.e. (subreg:V4SF (reg:SF) 0) or (subreg:V4SF (reg:V2SF) 0). This
surely isn't the cleanest way to represent this. It's questionable
if this ought to be represented at all -- why can't this all be hidden
in post-reload splitters that make arbitrarily mode changes to the
registers themselves. */
else if (VECTOR_MODE_P (omode)
&& GET_MODE_INNER (omode) == GET_MODE_INNER (imode))
;
/* Subregs involving floating point modes are not allowed to
change size. Therefore (subreg:DI (reg:DF) 0) is fine, but
(subreg:SI (reg:DF) 0) isn't. */
else if (FLOAT_MODE_P (imode) || FLOAT_MODE_P (omode))
{
if (! (known_eq (isize, osize)
/* LRA can use subreg to store a floating point value in
an integer mode. Although the floating point and the
integer modes need the same number of hard registers,
the size of floating point mode can be less than the
integer mode. LRA also uses subregs for a register
should be used in different mode in on insn. */
|| lra_in_progress))
return false;
}
altogether.
Thanks,
Richard
