| Message ID | 52D6BCFC.6000408@mentor.com |
|---|---|
| State | New |
| Headers | show |
Tom de Vries <Tom_deVries@mentor.com> writes: > This patch adds scanning of clobbers in CALL_INSN_FUNCTION_USAGE to > find_all_hard_reg_sets. > > For MIPS, calls are split at some point. After the split, one of the > resulting insns may clobber $6. But before the split, that's not > explicit in the rtl representation of the unsplit call. > > For -fuse-caller-save, that's a problem, and Richard S. suggested to > add the clobber of $6 to the CALL_INSN_FUNCTION_USAGE of the unsplit > call. Maybe here's a good place to raise this, but I was hoping all targets could use CALL_INSN_FUNCTION_USAGE and we'd be able to drop the new hook for -fuse-caller-save. That seems better than adding another place that needs to be checked when figuring out what a call clobbers. In other words, I didn't want MIPS to be different from the other targets. If we decide to keep the hook then MIPS should use it too. Thanks, Richard
On 15-01-14 19:40, Richard Sandiford wrote: > Tom de Vries <Tom_deVries@mentor.com> writes: >> This patch adds scanning of clobbers in CALL_INSN_FUNCTION_USAGE to >> find_all_hard_reg_sets. >> >> For MIPS, calls are split at some point. After the split, one of the >> resulting insns may clobber $6. But before the split, that's not >> explicit in the rtl representation of the unsplit call. >> >> For -fuse-caller-save, that's a problem, and Richard S. suggested to >> add the clobber of $6 to the CALL_INSN_FUNCTION_USAGE of the unsplit >> call. > > Maybe here's a good place to raise this, but I was hoping all targets > could use CALL_INSN_FUNCTION_USAGE and we'd be able to drop the new > hook for -fuse-caller-save. That seems better than adding another > place that needs to be checked when figuring out what a call clobbers. > Hi Richard, There are the following sets of registers involved: * CALL_REALLY_USED_REGS, which is the set normally clobbered by called functions (or PLTs, resolvers, or any other piece of code that runs during a function call) * The set of registers discovered by the use-caller-save optimization, which takes into account only the registers clobbered by the called function itself * The set of registers which are clobbered during a call by things like the plt - these are not picked up by the use-caller-save optimization. We need the hook to inform the compiler about these registers * And finally, registers clobbered in the caller itself during a sequence of instructions implementing a function call. On mips, that's R6, which may be clobbered by the call. Normally that doesn't need mentioning in the RTL since it's a call_used_reg, but since use-caller-save might discover a set of registers for the called function that does not include R6, it becomes important to record this clobber explicitly. It could be represented in the RTL by a clobber on the insn, or a clobber in C_I_F_U. Or it could just be part of the registers returned by the hook - but that was previously deemed not acceptable (and it doesn't match the description of the hook). > In other words, I didn't want MIPS to be different from the other targets. MIPS just needs some special handling for the unsplit call. There might be other targets which need similar tweaks. > If we decide to keep the hook then MIPS should use it too. > I think the current proposal is good: - use the hook to model registers clobbered during a call - use CALL_INSN_FUNCTION_USAGE to model the r6 clobber for the unsplit MIPS call_insn Thanks, - Tom > Thanks, > Richard >
Tom de Vries <Tom_deVries@mentor.com> writes: > * The set of registers which are clobbered during a call by things like the plt > - these are not picked up by the use-caller-save optimization. We need the > hook to inform the compiler about these registers Right, but... > * And finally, registers clobbered in the caller itself during a sequence of > instructions implementing a function call. On mips, that's R6, which may be > clobbered by the call. Normally that doesn't need mentioning in the RTL since > it's a call_used_reg, but since use-caller-save might discover a set of > registers for the called function that does not include R6, it becomes > important to record this clobber explicitly. It could be represented in the > RTL by a clobber on the insn, or a clobber in C_I_F_U. Or it could just be > part of the registers returned by the hook - but that was previously deemed > not acceptable (and it doesn't match the description of the hook). ...why do we need two different mechanisms to deal with these two? IMO the set recorded for the callee should contain what the callee instructions clobber and nothing else. CALL_INSN_FUNCTION_USAGE should contain everything clobbered by a call outside the callee, whether that's in the calling function itself, in a PLT, in a MIPS16 stub, or whatever. Thanks, Richard
On 15-01-14 17:53, Tom de Vries wrote: > Eric, > > This patch adds scanning of clobbers in CALL_INSN_FUNCTION_USAGE to > find_all_hard_reg_sets. > > For MIPS, calls are split at some point. After the split, one of the resulting > insns may clobber $6. But before the split, that's not explicit in the rtl > representation of the unsplit call. > > For -fuse-caller-save, that's a problem, and Richard S. suggested to add the > clobber of $6 to the CALL_INSN_FUNCTION_USAGE of the unsplit call. > > I wrote a patch for that ( > http://gcc.gnu.org/ml/gcc-patches/2014-01/msg00730.html ), but found that doing > so did not fix the problem with -fuse-caller-save, because > find_all_hard_reg_sets (the mechanism -fuse-caller-save uses to detect which > registers are set or clobbered) does not take CALL_INSN_FUNCTION_USAGE into > account. This patch fixes that. > > Build and reg-tested on MIPS. > > OK for stage1 if x86_64 bootstrap & reg-test succeeds? > Eric, Ping of this ( http://gcc.gnu.org/ml/gcc-patches/2014-01/msg00888.html ) patch. Ok for stage1? Thanks, - Tom
> Ping of this ( http://gcc.gnu.org/ml/gcc-patches/2014-01/msg00888.html ) > patch. That patch isn't for GCC mainline though, but OK on principle if you test it on mainline, avoid the very ugly set-inside-use idiom and do: record_hard_reg_sets (XEXP (op, 0), NULL, pset); instead of reimplementing it manually.
2014-01-15 Tom de Vries <tom@codesourcery.com> * rtlanal.c (find_all_hard_reg_sets): Note INSN_CALL_FUNCTION_USAGE clobbers. diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c index 7e27774..64f83ac 100644 --- a/gcc/rtlanal.c +++ b/gcc/rtlanal.c @@ -1034,8 +1034,25 @@ find_all_hard_reg_sets (const_rtx insn, HARD_REG_SET *pset, bool implicit) CLEAR_HARD_REG_SET (*pset); note_stores (PATTERN (insn), record_hard_reg_sets, pset); - if (implicit && CALL_P (insn)) - IOR_HARD_REG_SET (*pset, call_used_reg_set); + if (CALL_P (insn)) + { + if (implicit) + IOR_HARD_REG_SET (*pset, call_used_reg_set); + + for (link = CALL_INSN_FUNCTION_USAGE (insn); link; link = XEXP (link, 1)) + { + rtx op, reg; + + if (GET_CODE (op = XEXP (link, 0)) == CLOBBER + && REG_P (reg = XEXP (op, 0))) + { + unsigned int i; + + for (i = REGNO (reg) ; i < END_HARD_REGNO (reg); i++) + SET_HARD_REG_BIT (*pset, i); + } + } + } for (link = REG_NOTES (insn); link; link = XEXP (link, 1)) if (REG_NOTE_KIND (link) == REG_INC) record_hard_reg_sets (XEXP (link, 0), NULL, pset);