| Message ID | 52A33995.4090002@mentor.com |
|---|---|
| State | New |
| Headers | show |
On 07-12-13 16:07, Tom de Vries wrote: > Richard, > > This patch implements the target hook TARGET_FN_OTHER_HARD_REG_USAGE (posted > here: http://gcc.gnu.org/ml/gcc-patches/2013-03/msg01318.html) for MIPS, to > address the issue that $6 is sometimes used in split calls. > > Build and reg-tested on MIPS. > > OK for stage1? > Richard, Ping. This patch was submitted here ( http://gcc.gnu.org/ml/gcc-patches/2013-12/msg00771.html ) and is required for the -fuse-caller-save optimization which was submitted here ( http://gcc.gnu.org/ml/gcc-patches/2013-01/msg01234.html ). The patch fixes a correctness issue with -fuse-caller-save for MIPS. OK for stage1? Thanks, - Tom > Thanks, > - Tom >
On 25/12/13 14:02, Tom de Vries wrote: > On 07-12-13 16:07, Tom de Vries wrote: >> Richard, >> >> This patch implements the target hook TARGET_FN_OTHER_HARD_REG_USAGE (posted >> here: http://gcc.gnu.org/ml/gcc-patches/2013-03/msg01318.html) for MIPS, to >> address the issue that $6 is sometimes used in split calls. >> >> Build and reg-tested on MIPS. >> >> OK for stage1? >> > Richard, Ping. This patch is the only part of -fuse-caller-save that still needs approval. > This patch was submitted here ( > http://gcc.gnu.org/ml/gcc-patches/2013-12/msg00771.html ) and is required for > the -fuse-caller-save optimization which was submitted here ( > http://gcc.gnu.org/ml/gcc-patches/2013-01/msg01234.html ). > > The patch fixes a correctness issue with -fuse-caller-save for MIPS. > > OK for stage1? > Thanks, - Tom
Tom de Vries <Tom_deVries@mentor.com> writes: > On 25/12/13 14:02, Tom de Vries wrote: >> On 07-12-13 16:07, Tom de Vries wrote: >>> Richard, >>> >>> This patch implements the target hook TARGET_FN_OTHER_HARD_REG_USAGE (posted >>> here: http://gcc.gnu.org/ml/gcc-patches/2013-03/msg01318.html) for MIPS, to >>> address the issue that $6 is sometimes used in split calls. >>> >>> Build and reg-tested on MIPS. >>> >>> OK for stage1? >>> >> > > Richard, > > Ping. > > This patch is the only part of -fuse-caller-save that still needs approval. Hmm, where were parts 4 and 6 approved? Was looking for the discussion in the hope that it would answer the question I don't really understand, which is: this hook is only used during final, is that right? And the clobber that you're adding is exposed at the rtl level. So why do we need the hook at all? Why not just collect the usage information at the end of final rather than at the beginning, so that all splits during final have been done? For other cases (where the usage isn't explicit at the rtl level), why not record the usage in CALL_INSN_FUNCTION_USAGE instead? Thanks, Richard
On 09-01-14 16:31, Richard Sandiford wrote: > Tom de Vries <Tom_deVries@mentor.com> writes: >> On 25/12/13 14:02, Tom de Vries wrote: >>> On 07-12-13 16:07, Tom de Vries wrote: >>>> Richard, >>>> >>>> This patch implements the target hook TARGET_FN_OTHER_HARD_REG_USAGE (posted >>>> here: http://gcc.gnu.org/ml/gcc-patches/2013-03/msg01318.html) for MIPS, to >>>> address the issue that $6 is sometimes used in split calls. >>>> >>>> Build and reg-tested on MIPS. >>>> >>>> OK for stage1? >>>> >>> >> >> Richard, >> >> Ping. >> >> This patch is the only part of -fuse-caller-save that still needs approval. > Richard, thanks for the review. > Hmm, where were parts 4 and 6 approved? In http://gcc.gnu.org/ml/gcc-patches/2013-03/msg00508.html, Vladimir wrote: ... The patch is ok for me for trunk at stage1. But I think you need a formal approval for df-scan.c, arm.c, mips.c, GCC testsuite expect files (lib/target-supports.exp and gcc.target/mips/mips.exp) as I am not a maintainer of these parts although these changes look ok for me. ... In reaction to that, I split up the patch into a patches series, and replied in http://gcc.gnu.org/ml/gcc-patches/2013-03/msg01255.html: ... I'm assuming you've ok'ed patch 1, 2, 3, 4, 6, 8, 9 and the non-df-scan part of 7. I'll ask other maintainers about the other parts (5, 10 and the df-scan part of 7). ... > Was looking for the discussion > in the hope that it would answer the question I don't really understand, > which is: this hook is only used during final, is that right? Yes. > And the > clobber that you're adding is exposed at the rtl level. Yes, after the calls are split, but not before. > So why do we > need the hook at all? In general we need the hook for registers that are clobbered during a call to a function, while the registers are not present in the final rtl representation of that function. For MIPS, we don't need the hook for that purpose. But, for MIPS there's the following issue: the unsplit call clobbers r6, but the clobber is not explicit in the rtl. Only after splitting, the clobber becomes explicit in the rtl. In general, that's not a problem because r6 is a member of the set of register clobbered by a call (CALL_REALLY_USED_REGISTERS), so it's implicitly clobbered. But for -fuse-caller-save, when we find a call, we ignore CALL_REALLY_USED_REGISTERS and use a potentially smaller set of implicit clobbers: the union of: - the registers usage analysis of the final rtl representation of the called function - the registers marked by the hook. So before splitting the unsplit call, there's nothing to tell us that r6 is clobbered by that call. Resulting in register allocation using r6 as if it was not clobbered, which causes errors. > Why not just collect the usage information at > the end of final rather than at the beginning, so that all splits during > final have been done? If we have a call to a leaf function, the final rtl representation does not contain calls. The problem does not lie in the final pass where the callee is analyzed, but in the caller, where information is used, and where the unsplit call is missing the clobber of r6. > For other cases (where the usage isn't explicit > at the rtl level), why not record the usage in CALL_INSN_FUNCTION_USAGE > instead? > Right, we could add the r6 clobber that way. But to keep things simple, I've used the hook instead. Thanks, - Tom > Thanks, > Richard >
Tom de Vries <Tom_deVries@mentor.com> writes: >> Why not just collect the usage information at >> the end of final rather than at the beginning, so that all splits during >> final have been done? > > If we have a call to a leaf function, the final rtl representation does not > contain calls. The problem does not lie in the final pass where the callee is > analyzed, but in the caller, where information is used, and where the unsplit > call is missing the clobber of r6. Ah, so when you're using this hook in final, you're actually adding in the set of registers that will be clobbered by a future caller's CALL_INSN, as well as the registers that are clobbered by the callee itself? That seems a bit error-prone, since we don't know at this stage what the future caller will look like. (Things like the target attribute make this harder to predict.) I think it would be cleaner to just calculate the callee-clobbered registers during final and leave the caller to say what it clobbers. FWIW, I still think it'd be better to collect the set at the end of final (after any final splits) rather than at the beginning. >> For other cases (where the usage isn't explicit >> at the rtl level), why not record the usage in CALL_INSN_FUNCTION_USAGE >> instead? >> > > Right, we could add the r6 clobber that way. But to keep things simple, I've > used the hook instead. Why's it simpler though? That's the kind of thing CALL_INSN_FUNCTION_USAGE is there for. Thanks, Richard
2013-11-12 Chung-Lin Tang <cltang@codesourcery.com>
Tom de Vries <tom@codesourcery.com>
* config/mips/mips.c (POST_CALL_TMP_REG): Define.
(mips_split_call): Use POST_CALL_TMP_REG.
(mips_fn_other_hard_reg_usage): New function.
(TARGET_FN_OTHER_HARD_REG_USAGE): Define targhook using new function.
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 36ba6df..3f60f5b 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -175,6 +175,11 @@ along with GCC; see the file COPYING3. If not see
/* Return the usual opcode for a nop. */
#define MIPS_NOP 0
+/* Temporary register that is used after a call, and suitable for both
+ MIPS16 and non-MIPS16 code. $4 and $5 are used for returning complex double
+ values in soft-float code, so $6 is the first suitable candidate. */
+#define POST_CALL_TMP_REG (GP_ARG_FIRST + 2)
+
/* Classifies an address.
ADDRESS_REG
@@ -6990,10 +6995,8 @@ mips_split_call (rtx insn, rtx call_pattern)
{
emit_call_insn (call_pattern);
if (!find_reg_note (insn, REG_NORETURN, 0))
- /* Pick a temporary register that is suitable for both MIPS16 and
- non-MIPS16 code. $4 and $5 are used for returning complex double
- values in soft-float code, so $6 is the first suitable candidate. */
- mips_restore_gp_from_cprestore_slot (gen_rtx_REG (Pmode, GP_ARG_FIRST + 2));
+ mips_restore_gp_from_cprestore_slot (gen_rtx_REG (Pmode,
+ POST_CALL_TMP_REG));
}
/* Return true if a call to DECL may need to use JALX. */
@@ -18699,6 +18702,32 @@ mips_case_values_threshold (void)
else
return default_case_values_threshold ();
}
+
+/* Implement TARGET_FN_OTHER_HARD_REG_USAGE. */
+
+static void
+mips_fn_other_hard_reg_usage (struct hard_reg_set_container *fn_used_regs)
+{
+ /* POST_CALL_TMP_REG is used in splitting calls after register allocation.
+ With -fno-use-caller-save, the register is available because register
+ allocation ensures that members of call_used_regs are not live across
+ calls.
+ With -fuse-caller-save that's not the case, so we're missing a clobber on
+ the unsplit call insn to tell register allocation that the register is used
+ by the split call insn(s) after register allocation (we don't need the
+ clobber for a non-returning call, but we don't expect there will be a
+ penalty if we add the clobber for both returning and non-returning calls).
+
+ For the sake of simplicity we don't add the individual clobbers, but we use
+ this hook to mark the reg as clobbered. This is a bit ugly, since this
+ hook is called during the final pass on a function, and we're expressing
+ here that the insn after a call to this function will clobber a register.
+
+ The condition is the pass-independent part of TARGET_SPLIT_CALLS. */
+ if (TARGET_EXPLICIT_RELOCS
+ && TARGET_CALL_CLOBBERED_GP)
+ SET_HARD_REG_BIT (fn_used_regs->set, POST_CALL_TMP_REG);
+}
/* Initialize the GCC target structure. */
#undef TARGET_ASM_ALIGNED_HI_OP
@@ -18933,6 +18962,9 @@ mips_case_values_threshold (void)
#undef TARGET_CASE_VALUES_THRESHOLD
#define TARGET_CASE_VALUES_THRESHOLD mips_case_values_threshold
+#undef TARGET_FN_OTHER_HARD_REG_USAGE
+#define TARGET_FN_OTHER_HARD_REG_USAGE mips_fn_other_hard_reg_usage
+
struct gcc_target targetm = TARGET_INITIALIZER;
#include "gt-mips.h"