Alex Coplan <alex.cop...@arm.com> writes: > Hi, > > For the testcase in the PR, we try to pair insns where the first has > writeback and the second uses the updated base register. This causes us > to record a hazard against the second insn, thus narrowing the move > range away from the end of the BB. > > However, it isn't meaningful to record hazards against the other insn > in the pair, as this doesn't change which pairs can be formed, and also > doesn't change where the pair is formed (from the perspective of > nondebug insns). > > To see why this is the case, consider the two cases: > > - Suppoe we are finding hazards for insns[0]. If we record a hazard > against insns[1], then range.last becomes > insns[1]->prev_nondebug_insn (), but note that this is equivalent to > inserting after insns[1] (since insns[1] is being changed). > - Now consider finding hazards for insns[1]. Suppose we record > insns[0] as a hazard. Then we set range.first = insns[0], which is a > no-op. > > As such, it seems better to never record hazards against the other insn > in the pair, as we check whether the insns themselves are suitable for > combination separately (e.g. for ldp checking that they use distinct > transfer registers). Avoiding unnecessarily narrowing the move range > avoids unnecessarily re-ordering over debug insns. > > This should also mean that we can only narrow the move range away from > the end of the BB in the case that we record a hazard for insns[0] > against insns[1]->prev_nondebug_insn () or earlier. This means that for > the non-call-exceptions case, either the move range includes insns[1], > or we reject the pair (thus the assert tripped in the PR should always > hold). > > Bootstrapped/regtested on aarch64-linux-gnu with/without ldp passes > enabled on top of the PR113070 fixes, OK for trunk? > > Thanks, > Alex > > gcc/ChangeLog: > > PR target/113356 > * config/aarch64/aarch64-ldp-fusion.cc (ldp_bb_info::try_fuse_pair): > Don't record hazards against the opposite insn in the pair. > > gcc/testsuite/ChangeLog: > > PR target/113356 > * gcc.target/aarch64/pr113356.C: New test.
OK, thanks. Richard
