| Message ID | ZaVAeoKV0dHc6Wk1@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: Don't record hazards against paired insns [PR113356] | expand |
Alex Coplan <alex.coplan@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
diff --git a/gcc/config/aarch64/aarch64-ldp-fusion.cc b/gcc/config/aarch64/aarch64-ldp-fusion.cc index 703cfb1228c..6834560c5fb 100644 --- a/gcc/config/aarch64/aarch64-ldp-fusion.cc +++ b/gcc/config/aarch64/aarch64-ldp-fusion.cc @@ -2216,11 +2216,11 @@ ldp_bb_info::try_fuse_pair (bool load_p, unsigned access_size, ignore[j] = &XEXP (cand_mems[j], 0); insn_info *h = first_hazard_after (insns[0], ignore[0]); - if (h && *h <= *insns[1]) + if (h && *h < *insns[1]) cand.hazards[0] = h; h = latest_hazard_before (insns[1], ignore[1]); - if (h && *h >= *insns[0]) + if (h && *h > *insns[0]) cand.hazards[1] = h; if (!cand.viable ()) diff --git a/gcc/testsuite/gcc.target/aarch64/pr113356.C b/gcc/testsuite/gcc.target/aarch64/pr113356.C new file mode 100644 index 00000000000..0de17a54a53 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/pr113356.C @@ -0,0 +1,8 @@ +// { dg-do compile } +// { dg-options "-Os -fnon-call-exceptions -mearly-ldp-fusion -fno-lifetime-dse -fno-forward-propagate" } +struct Class1 { + virtual ~Class1() {} + unsigned Field1; +}; +struct Class4 : virtual Class1 {}; +int main() { Class4 var1; }