On Thu, Aug 26, 2021 at 7:09 PM Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > On Thu, Aug 26, 2021 at 12:50 PM Richard Sandiford > <richard.sandif...@arm.com> wrote: > > > > 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. > > Yeah, I would fully support this. Maybe replace it with a comment > but I don't know what it should say. > > Richard. > > > > > Thanks, > > Richard
I'm going to upstream the patch. -- BR, Hongtao