On Fri, Sep 10, 2021 at 5:07 PM Segher Boessenkool <seg...@kernel.crashing.org> wrote: > > On Fri, Sep 10, 2021 at 12:53:37PM +0200, Richard Biener wrote: > > On Fri, Sep 10, 2021 at 1:50 AM Segher Boessenkool > > <seg...@kernel.crashing.org> wrote: > > > And many targets have strange rules for bit-strings in which modes can > > > be used as bit-strings in which other modes, and at what offsets in > > > which registers. Now perhaps none of that is optimal (I bet it isn't), > > > but changing this without a transition plan simply does not work. > > > > But we _do_ already allow some of them :/ Like > > Yes. And all of this is old and ingrained, and targets depend on the > status quo, so changing this will need more care and planning and > cooperation. It certainly is a worthwhile thing to improve, but it is > not a small project, and it requires a plan. > > > /* ??? 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)) > > ; > > > > so for the special case where 'regsize' matches osize it would be > > a bit-cast of a full register from int to float. But as written it also > > allows (subreg:XF (reg:TI)) which will likely wreck havoc? > > That does not pass the isize >= osize test? Or maybe I don't know > what XFmode is well enough :-) Hey I can read, I have source, and it > is Friday...
TImode is 16 bytes but XFmode is 12. I meant to construct a case which passes all the tests but where definitely such kind of subreg is very odd to be "allowed" by validate_subreg - a target may of course have means to make sense of it (I don't see how x87 would though). > Ah. So XF has different size on 32-bit and on 64-bit, but that doesn't > even matter here. > > > Similar for the omode == word_mode check which allows > > (subreg:DI (reg:TF ..)). That is, the existing special-cases look > > too broad to me - and they probably exist because when validate_subreg > > rejects sth then we can't put it together later when expand split it > > into two subregs and a pseudo ... > > I said it before, and I'll say it again, it is a very important point: > expand should not try to optimise this, *at all*. And not just this, > not *anything*. Expand's job in the current compiler is only to > translate Gimple to RTL, and nothing more. +1 (or +10!), but unfortunately expand _is_ an important part of optimization - in particular when it gets to avoiding stack usage since we can never get rid of all effects of allocating a stack slot. Splitting to multiple insns with pseudos should be fine though, but it at least seems that splitting (subreg:FLOAT (reg:INT)) into (subreg:INT (reg:INT)) (subreg:FLOAT (reg:INT)) isn't always valid. Richard.
