| Message ID | mptblvqy7k9.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support multiple ABIs in the same translation unit | expand |
On Wed, Sep 11, 2019 at 08:08:38PM +0100, Richard Sandiford wrote: > hard_reg_set_iterator hrsi; > - EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, i, hrsi) > + EXECUTE_IF_SET_IN_HARD_REG_SET (abi.full_and_partial_reg_clobbers (), > + 0, i, hrsi) So "abi" in that means calls? It is not such a great name like that. Since its children are very_long_names, it doesn't need to be only three chars itself, either? Segher
Segher Boessenkool <segher@kernel.crashing.org> writes: > On Wed, Sep 11, 2019 at 08:08:38PM +0100, Richard Sandiford wrote: >> hard_reg_set_iterator hrsi; >> - EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, i, hrsi) >> + EXECUTE_IF_SET_IN_HARD_REG_SET (abi.full_and_partial_reg_clobbers (), >> + 0, i, hrsi) > > So "abi" in that means calls? "abi" is the interface of the callee function, taking things like function attributes and -fipa-ra into account. The register sets are describing what the callee does rather than what calls to it do. E.g. on targets that allow linker stubs to be inserted between calls, the scratch registers reserved for linker stubs are still call-clobbered, even if the target of the call doesn't use them. (Those call clobbers are represented separately, at least when TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS is true. When it's false we don't use -fipa-ra information at all.) > It is not such a great name like that. Since its children are > very_long_names, it doesn't need to be only three chars itself, > either? OK, what name would you prefer? Richard
Hi Richard, Sorry this too me so long to get back to. On Thu, Sep 12, 2019 at 08:51:59AM +0100, Richard Sandiford wrote: > Segher Boessenkool <segher@kernel.crashing.org> writes: > > On Wed, Sep 11, 2019 at 08:08:38PM +0100, Richard Sandiford wrote: > >> hard_reg_set_iterator hrsi; > >> - EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, i, hrsi) > >> + EXECUTE_IF_SET_IN_HARD_REG_SET (abi.full_and_partial_reg_clobbers (), > >> + 0, i, hrsi) > > > > So "abi" in that means calls? > > "abi" is the interface of the callee function, taking things like > function attributes and -fipa-ra into account. > > The register sets are describing what the callee does rather than > what calls to it do. E.g. on targets that allow linker stubs to be > inserted between calls, the scratch registers reserved for linker stubs > are still call-clobbered, even if the target of the call doesn't use > them. (Those call clobbers are represented separately, at least when > TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS is true. When it's > false we don't use -fipa-ra information at all.) > > > It is not such a great name like that. Since its children are > > very_long_names, it doesn't need to be only three chars itself, > > either? > > OK, what name would you prefer? Maybe call_abi is a good name? It's difficult to capture the subtleties in a short enough name. As always :-) Segher
Segher Boessenkool <segher@kernel.crashing.org> writes: > Hi Richard, > > Sorry this too me so long to get back to. > > On Thu, Sep 12, 2019 at 08:51:59AM +0100, Richard Sandiford wrote: >> Segher Boessenkool <segher@kernel.crashing.org> writes: >> > On Wed, Sep 11, 2019 at 08:08:38PM +0100, Richard Sandiford wrote: >> >> hard_reg_set_iterator hrsi; >> >> - EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, i, hrsi) >> >> + EXECUTE_IF_SET_IN_HARD_REG_SET (abi.full_and_partial_reg_clobbers (), >> >> + 0, i, hrsi) >> > >> > So "abi" in that means calls? >> >> "abi" is the interface of the callee function, taking things like >> function attributes and -fipa-ra into account. >> >> The register sets are describing what the callee does rather than >> what calls to it do. E.g. on targets that allow linker stubs to be >> inserted between calls, the scratch registers reserved for linker stubs >> are still call-clobbered, even if the target of the call doesn't use >> them. (Those call clobbers are represented separately, at least when >> TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS is true. When it's >> false we don't use -fipa-ra information at all.) >> >> > It is not such a great name like that. Since its children are >> > very_long_names, it doesn't need to be only three chars itself, >> > either? >> >> OK, what name would you prefer? > > Maybe call_abi is a good name? It's difficult to capture the subtleties > in a short enough name. As always :-) The formatting ended up being a bit weird with a longer name, so how about the attached instead? Richard 2019-09-25 Richard Sandiford <richard.sandiford@arm.com> gcc/ * combine.c: Include function-abi.h. (record_dead_and_set_regs): Use insn_callee_abi to get the ABI of the target of call insns. Invalidate partially-clobbered registers as well as fully-clobbered ones. Index: gcc/combine.c =================================================================== --- gcc/combine.c 2019-09-12 10:52:53.000000000 +0100 +++ gcc/combine.c 2019-09-25 16:50:21.772865265 +0100 @@ -105,6 +105,7 @@ Software Foundation; either version 3, o #include "valtrack.h" #include "rtl-iter.h" #include "print-rtl.h" +#include "function-abi.h" /* Number of attempts to combine instructions in this function. */ @@ -13464,11 +13465,21 @@ record_dead_and_set_regs (rtx_insn *insn if (CALL_P (insn)) { + HARD_REG_SET callee_clobbers + = insn_callee_abi (insn).full_and_partial_reg_clobbers (); hard_reg_set_iterator hrsi; - EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, i, hrsi) + EXECUTE_IF_SET_IN_HARD_REG_SET (callee_clobbers, 0, i, hrsi) { reg_stat_type *rsp; + /* ??? We could try to preserve some information from the last + set of register I if the call doesn't actually clobber + (reg:last_set_mode I), which might be true for ABIs with + partial clobbers. However, it would be difficult to + update last_set_nonzero_bits and last_sign_bit_copies + to account for the part of I that actually was clobbered. + It wouldn't help much anyway, since we rarely see this + situation before RA. */ rsp = ®_stat[i]; rsp->last_set_invalid = 1; rsp->last_set = insn;
On Wed, Sep 25, 2019 at 04:52:14PM +0100, Richard Sandiford wrote: > Segher Boessenkool <segher@kernel.crashing.org> writes: > > On Thu, Sep 12, 2019 at 08:51:59AM +0100, Richard Sandiford wrote: > >> Segher Boessenkool <segher@kernel.crashing.org> writes: > >> > It is not such a great name like that. Since its children are > >> > very_long_names, it doesn't need to be only three chars itself, > >> > either? > >> > >> OK, what name would you prefer? > > > > Maybe call_abi is a good name? It's difficult to capture the subtleties > > in a short enough name. As always :-) > > The formatting ended up being a bit weird with a longer name, > so how about the attached instead? That looks great, thanks! > + /* ??? We could try to preserve some information from the last > + set of register I if the call doesn't actually clobber > + (reg:last_set_mode I), which might be true for ABIs with > + partial clobbers. However, it would be difficult to > + update last_set_nonzero_bits and last_sign_bit_copies > + to account for the part of I that actually was clobbered. > + It wouldn't help much anyway, since we rarely see this > + situation before RA. */ I would like to completely get rid of reg_stat, and have known bits dealt with by some DF thing instead... It would work much better and be much easier to use at the same time. Also, other passes could use it as well. If I ever will find time to do this, I don't know :-/ Segher
On 9/25/19 9:52 AM, Richard Sandiford wrote: > Segher Boessenkool <segher@kernel.crashing.org> writes: >> Hi Richard, >> >> Sorry this too me so long to get back to. >> >> On Thu, Sep 12, 2019 at 08:51:59AM +0100, Richard Sandiford wrote: >>> Segher Boessenkool <segher@kernel.crashing.org> writes: >>>> On Wed, Sep 11, 2019 at 08:08:38PM +0100, Richard Sandiford wrote: >>>>> hard_reg_set_iterator hrsi; >>>>> - EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, i, hrsi) >>>>> + EXECUTE_IF_SET_IN_HARD_REG_SET (abi.full_and_partial_reg_clobbers (), >>>>> + 0, i, hrsi) >>>> >>>> So "abi" in that means calls? >>> >>> "abi" is the interface of the callee function, taking things like >>> function attributes and -fipa-ra into account. >>> >>> The register sets are describing what the callee does rather than >>> what calls to it do. E.g. on targets that allow linker stubs to be >>> inserted between calls, the scratch registers reserved for linker stubs >>> are still call-clobbered, even if the target of the call doesn't use >>> them. (Those call clobbers are represented separately, at least when >>> TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS is true. When it's >>> false we don't use -fipa-ra information at all.) >>> >>>> It is not such a great name like that. Since its children are >>>> very_long_names, it doesn't need to be only three chars itself, >>>> either? >>> >>> OK, what name would you prefer? >> >> Maybe call_abi is a good name? It's difficult to capture the subtleties >> in a short enough name. As always :-) > > The formatting ended up being a bit weird with a longer name, > so how about the attached instead? > > Richard > > > 2019-09-25 Richard Sandiford <richard.sandiford@arm.com> > > gcc/ > * combine.c: Include function-abi.h. > (record_dead_and_set_regs): Use insn_callee_abi to get the ABI > of the target of call insns. Invalidate partially-clobbered > registers as well as fully-clobbered ones. OK if Segher doesn't object. jeff >
On Sun, Sep 29, 2019 at 04:32:13PM -0600, Jeff Law wrote: > On 9/25/19 9:52 AM, Richard Sandiford wrote: > > gcc/ > > * combine.c: Include function-abi.h. > > (record_dead_and_set_regs): Use insn_callee_abi to get the ABI > > of the target of call insns. Invalidate partially-clobbered > > registers as well as fully-clobbered ones. > OK if Segher doesn't object. https://gcc.gnu.org/ml/gcc-patches/2019-09/msg01462.html "That looks great, thanks!" :-) Segher
Index: gcc/combine.c =================================================================== --- gcc/combine.c 2019-09-09 18:58:51.448270881 +0100 +++ gcc/combine.c 2019-09-11 19:47:57.062032638 +0100 @@ -105,6 +105,7 @@ Software Foundation; either version 3, o #include "valtrack.h" #include "rtl-iter.h" #include "print-rtl.h" +#include "function-abi.h" /* Number of attempts to combine instructions in this function. */ @@ -13464,11 +13465,21 @@ record_dead_and_set_regs (rtx_insn *insn if (CALL_P (insn)) { + function_abi abi = call_insn_abi (insn); hard_reg_set_iterator hrsi; - EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, i, hrsi) + EXECUTE_IF_SET_IN_HARD_REG_SET (abi.full_and_partial_reg_clobbers (), + 0, i, hrsi) { reg_stat_type *rsp; + /* ??? We could try to preserve some information from the last + set of register I if the call doesn't actually clobber + (reg:last_set_mode I), which might be true for ABIs with + partial clobbers. However, it would be difficult to + update last_set_nonzero_bits and last_sign_bit_copies + to account for the part of I that actually was clobbered. + It wouldn't help much anyway, since we rarely see this + situation before RA. */ rsp = ®_stat[i]; rsp->last_set_invalid = 1; rsp->last_set = insn;