| Message ID | ZZfso6Yu4sBf5fzh@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: Further fix for throwing insns in ldp/stp pass [PR113217] | expand |
Alex Coplan <alex.coplan@arm.com> writes: > As the PR shows, the fix in > r14-6916-g057dc349021660c40699fb5c98fd9cac8e168653 was not complete. > That fix was enough to stop us trying to move throwing accesses above > nondebug insns, but due to this code in try_fuse_pair: > > // Placement strategy: push loads down and pull stores up, this should > // help register pressure by reducing live ranges. > if (load_p) > range.first = range.last; > else > range.last = range.first; > > we would still try to move stores up above any debug insns that occurred > immediately after the previous nondebug insn. This patch fixes that by > narrowing the move range in the case that the second access is throwing > to exactly the range of that insn. > > Note that we still need the fix to latest_hazard_before mentioned above > so as to ensure we select a suitable base and reject pairs if it isn't > viable to form the pair at the end of the BB. > > Bootstrapped/regtested on aarch64-linux-gnu, OK for trunk? > > Thanks, > Alex > > gcc/ChangeLog: > > PR target/113217 > * config/aarch64/aarch64-ldp-fusion.cc > (ldp_bb_info::try_fuse_pair): If the second access can throw, > narrow the move range to exactly that insn. > > gcc/testsuite/ChangeLog: > > PR target/113217 > * g++.dg/pr113217.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 25f9b2d01c5..2fe1b1d4d84 100644 > --- a/gcc/config/aarch64/aarch64-ldp-fusion.cc > +++ b/gcc/config/aarch64/aarch64-ldp-fusion.cc > @@ -2195,6 +2195,15 @@ ldp_bb_info::try_fuse_pair (bool load_p, unsigned access_size, > if (base->hazards[0]) > range.last = base->hazards[0]->prev_nondebug_insn (); > > + // If the second insn can throw, narrow the move range to exactly that insn. > + // This prevents us trying to move the second insn from the end of the BB. > + if (cfun->can_throw_non_call_exceptions > + && find_reg_note (insns[1]->rtl (), REG_EH_REGION, NULL_RTX)) > + { > + gcc_assert (range.includes (insns[1])); > + range = insn_range_info (insns[1]); > + } > + > // Placement strategy: push loads down and pull stores up, this should > // help register pressure by reducing live ranges. > if (load_p) > diff --git a/gcc/testsuite/g++.dg/pr113217.C b/gcc/testsuite/g++.dg/pr113217.C > new file mode 100644 > index 00000000000..ec861543930 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/pr113217.C > @@ -0,0 +1,15 @@ > +// { dg-do compile } > +// { dg-options "-O -g -fnon-call-exceptions" } > +struct _Vector_base { > + int _M_end_of_storage; > +}; > +struct vector : _Vector_base { > + vector() : _Vector_base() {} > + ~vector(); > +}; > +struct LoadGraph { > + LoadGraph(); > + vector colors; > + vector data_block; > +}; > +LoadGraph::LoadGraph() {}
diff --git a/gcc/config/aarch64/aarch64-ldp-fusion.cc b/gcc/config/aarch64/aarch64-ldp-fusion.cc index 25f9b2d01c5..2fe1b1d4d84 100644 --- a/gcc/config/aarch64/aarch64-ldp-fusion.cc +++ b/gcc/config/aarch64/aarch64-ldp-fusion.cc @@ -2195,6 +2195,15 @@ ldp_bb_info::try_fuse_pair (bool load_p, unsigned access_size, if (base->hazards[0]) range.last = base->hazards[0]->prev_nondebug_insn (); + // If the second insn can throw, narrow the move range to exactly that insn. + // This prevents us trying to move the second insn from the end of the BB. + if (cfun->can_throw_non_call_exceptions + && find_reg_note (insns[1]->rtl (), REG_EH_REGION, NULL_RTX)) + { + gcc_assert (range.includes (insns[1])); + range = insn_range_info (insns[1]); + } + // Placement strategy: push loads down and pull stores up, this should // help register pressure by reducing live ranges. if (load_p) diff --git a/gcc/testsuite/g++.dg/pr113217.C b/gcc/testsuite/g++.dg/pr113217.C new file mode 100644 index 00000000000..ec861543930 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr113217.C @@ -0,0 +1,15 @@ +// { dg-do compile } +// { dg-options "-O -g -fnon-call-exceptions" } +struct _Vector_base { + int _M_end_of_storage; +}; +struct vector : _Vector_base { + vector() : _Vector_base() {} + ~vector(); +}; +struct LoadGraph { + LoadGraph(); + vector colors; + vector data_block; +}; +LoadGraph::LoadGraph() {}
As the PR shows, the fix in r14-6916-g057dc349021660c40699fb5c98fd9cac8e168653 was not complete. That fix was enough to stop us trying to move throwing accesses above nondebug insns, but due to this code in try_fuse_pair: // Placement strategy: push loads down and pull stores up, this should // help register pressure by reducing live ranges. if (load_p) range.first = range.last; else range.last = range.first; we would still try to move stores up above any debug insns that occurred immediately after the previous nondebug insn. This patch fixes that by narrowing the move range in the case that the second access is throwing to exactly the range of that insn. Note that we still need the fix to latest_hazard_before mentioned above so as to ensure we select a suitable base and reject pairs if it isn't viable to form the pair at the end of the BB. Bootstrapped/regtested on aarch64-linux-gnu, OK for trunk? Thanks, Alex gcc/ChangeLog: PR target/113217 * config/aarch64/aarch64-ldp-fusion.cc (ldp_bb_info::try_fuse_pair): If the second access can throw, narrow the move range to exactly that insn. gcc/testsuite/ChangeLog: PR target/113217 * g++.dg/pr113217.C: New test.