(sorry repost due to the lack of cc here) Hi! On 2022/08/04 18:49, Richard Sandiford wrote: > Takayuki 'January June' Suwa <jjsuwa_sys3...@yahoo.co.jp> writes: >> Thanks for your response. >> >> On 2022/08/03 16:52, Richard Sandiford wrote: >>> Takayuki 'January June' Suwa via Gcc-patches <gcc-patches@gcc.gnu.org> >>> writes: >>>> Emitting "(clobber (reg X))" before "(set (subreg (reg X)) (...))" keeps >>>> data flow consistent, but it also increases register allocation pressure >>>> and thus often creates many unwanted register-to-register moves that >>>> cannot be optimized away. >>> >>> There are two things here: >>> >>> - If emit_move_complex_parts emits a clobber of a hard register, >>> then that's probably a bug/misfeature. The point of the clobber is >>> to indicate that the register has no useful contents. That's useful >>> for wide pseudos that are written to in parts, since it avoids the >>> need to track the liveness of each part of the pseudo individually. >>> But it shouldn't be necessary for hard registers, since subregs of >>> hard registers are simplified to hard registers wherever possible >>> (which on most targets is "always"). >>> >>> So I think the emit_move_complex_parts clobber should be restricted >>> to !HARD_REGISTER_P, like the lower-subreg clobber is. If that helps >>> (if only partly) then it would be worth doing as its own patch. >>> >>> - I think it'd be worth looking into more detail why a clobber makes >>> a difference to register pressure. A clobber of a pseudo register R >>> shouldn't make R conflict with things that are live at the point of >>> the clobber. >> >> I agree with its worth. >> In fact, aside from other ports, on the xtensa one, RA in code with frequent >> D[FC]mode pseudos is terribly bad. >> For example, in __muldc3 on libgcc2, the size of the stack frame reserved >> will almost double depending on whether or not this patch is applied. > > Yeah, that's a lot.
So lots, but almost double might be an overstatement :) BTW after some quick experimentation, I found that turning on -fsplit-wide-types-early would roughly (but not completely) solve the problem. Surely, the output was not so bad in the past... > >>>> It seems just analogous to partial register >>>> stall which is a famous problem on processors that do register renaming. >>>> >>>> In my opinion, when the register to be clobbered is a composite of hard >>>> ones, we should clobber the individual elements separetely, otherwise >>>> clear the entire to zero prior to use as the "init-regs" pass does (like >>>> partial register stall workarounds on x86 CPUs). Such redundant zero >>>> constant assignments will be removed later in the "cprop_hardreg" pass. >>> >>> I don't think we should rely on the zero being optimised away later. >>> >>> Emitting the zero also makes it harder for the register allocator >>> to elide the move. For example, if we have: >>> >>> (set (subreg:SI (reg:DI P) 0) (reg:SI R0)) >>> (set (subreg:SI (reg:DI P) 4) (reg:SI R1)) >>> >>> then there is at least a chance that the RA could assign hard registers >>> R0:R1 to P, which would turn the moves into nops. If we emit: >>> >>> (set (reg:DI P) (const_int 0)) >>> >>> beforehand then that becomes impossible, since R0 and R1 would then >>> conflict with P. >> >> Ah, surely, as you pointed out for targets where "(reg: DI)" corresponds to >> one hard register. > > I was thinking here about the case where (reg:DI …) corresponds to > 2 hard registers. Each subreg move is then a single hard register > copy, but assigning P to the combination R0:R1 can remove both of > the subreg moves. > >>> TBH I'm surprised we still run init_regs for LRA. I thought there was >>> a plan to stop doing that, but perhaps I misremember. >> >> Sorry I am not sure about the status of LRA... because the xtensa port is >> still using reload. > > Ah, hadn't realised that. If you have time to work on it, it would be > really good to move over to LRA. There are plans to remove old reload. Alas you do overestimate me :) I've only been working about the GCC development for a little over a year. Well it's a lie that I'm not interested in it, but too much for me. > > It might be that old reload *does* treat a pseudo clobber as a conflict. > I can't remember now. If so, then zeroing the register wouldn't be > too bad (for old reload only). > >> As conclusion, trying to tweak the common code side may have been a bit >> premature. >> I'll consider if I can deal with those issues on the side of the >> target-specific code. > > It's likely to be at least partly a target-independent issue, so tweaking > the common code makes sense in principle. > > Does adding !HARD_REGISTER_P (x) to: > > /* Show the output dies here. This is necessary for SUBREGs > of pseudos since we cannot track their lifetimes correctly; > hard regs shouldn't appear here except as return values. */ > if (!reload_completed && !reload_in_progress > && REG_P (x) && !reg_overlap_mentioned_p (x, y)) > emit_clobber (x); > > in emit_move_complex_parts help? If so, I think we should do at Probably yes. Quick test says the abovementioned mod makes the ad-hoc fix I posted earlier (https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596626.html) a thing of the past. > least that much. > > Thanks, > Richard
