Alex Coplan <alex.cop...@arm.com> writes: > On 17/02/2025 16:15, Richard Sandiford wrote: >> Alex Coplan <alex.cop...@arm.com> writes: >> >> @@ -588,6 +590,10 @@ latest_hazard_before (insn_info *insn, rtx *ignore, >> >> && find_reg_note (insn->rtl (), REG_EH_REGION, NULL_RTX)) >> >> return insn->prev_nondebug_insn (); >> >> >> >> + if (!is_load_store >> >> + && accesses_include_memory (insn->defs ())) >> >> + return insn->prev_nondebug_insn (); >> > >> > This seems like it might be a little too restrictive. I agree that it's >> > a nice and simple way of solving the problem, but wouldn't it be enough >> > to prevent moving such accesses (stack deallocations) above the latest >> > preceding def or use of mem? Certainly we don't want to start >> > attempting alias analysis here, but is the above suggestion not a happy >> > middle ground (between a simple solution and not overly restricting >> > optimisation)? >> >> Would it help in practice though? Although it is possible to combine >> a deallocation with preceding stores, that only happens for dead code, >> in which case the better optimisation is to delete the stores. >> If we're combining with loads, the loads would normally be restoring >> registers for the caller, in which case the loads could be moved >> forward to the deallocation (since nothing would use or clobber >> the loaded values between the two points). > > I see. I must admit that I don't immediately see why this can only > occur with dead stores, [...]
I was thinking of the post-increment case, but yeah, I suppose technically there could be pre-increment cases. It seems very unlikely in practice, given how we manage the frame, but I agree that the case for not trying harder is weaker than I'd initially assumed. Thanks, Richard
