From 4eac9c2b0207ed53bc35898af9586987ee7733c9 Mon Sep 17 00:00:00 2001 From: Oleg Endo Date: Sun, 12 Oct 2014 23:14:07 +0000 Subject: [PATCH] re PR target/59401 ([SH] GBR addressing mode optimization produces wrong code) gcc/ PR target/59401 * config/sh/sh-protos (sh_find_equiv_gbr_addr): Use rtx_insn* instead of rtx. * config/sh/sh.c (sh_find_equiv_gbr_addr): Use def chains instead of insn walking. (sh_find_equiv_gbr_addr): Do nothing if input mem is already a GBR address. Use def chains to handle GBR clobbering call insns. gcc/testsuite/ PR target/59401 PR target/54760 * gcc.target/pr54760-5.c: New. * gcc.target/pr54760-6.c: New. * gcc.target/sh/pr59401-1.c: New. From-SVN: r216128 --- gcc/ChangeLog | 14 ++++- gcc/config/sh/sh-protos.h | 2 +- gcc/config/sh/sh.c | 83 +++++++++++++++++-------- gcc/testsuite/ChangeLog | 8 +++ gcc/testsuite/gcc.target/sh/pr54760-5.c | 26 ++++++++ gcc/testsuite/gcc.target/sh/pr54760-6.c | 19 ++++++ gcc/testsuite/gcc.target/sh/pr59401-1.c | 20 ++++++ 7 files changed, 144 insertions(+), 28 deletions(-) create mode 100644 gcc/testsuite/gcc.target/sh/pr54760-5.c create mode 100644 gcc/testsuite/gcc.target/sh/pr54760-6.c create mode 100644 gcc/testsuite/gcc.target/sh/pr59401-1.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 387e175f398..2ce1ca6823a 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,6 +1,16 @@ +2014-10-12 Oleg Endo + + PR target/59401 + * config/sh/sh-protos (sh_find_equiv_gbr_addr): Use rtx_insn* instead + of rtx. + * config/sh/sh.c (sh_find_equiv_gbr_addr): Use def chains instead of + insn walking. + (sh_find_equiv_gbr_addr): Do nothing if input mem is already a GBR + address. Use def chains to handle GBR clobbering call insns. + 2014-10-12 Trevor Saunders -* asan.c, cfgloop.c, cfgloop.h, cgraph.c, cgraph.h, + * asan.c, cfgloop.c, cfgloop.h, cgraph.c, cgraph.h, config/darwin.c, config/m32c/m32c.c, config/mep/mep.c, config/mips/mips.c, config/rs6000/rs6000.c, dwarf2out.c, function.c, function.h, gimple-ssa.h, libfuncs.h, optabs.c, @@ -32,7 +42,7 @@ 2014-10-11 Martin Liska - PR/63376 + PR middle-end/63376 * cgraphunit.c (symbol_table::process_new_functions): Missing call for call_cgraph_insertion_hooks added. diff --git a/gcc/config/sh/sh-protos.h b/gcc/config/sh/sh-protos.h index 336aaef6b8f..0839e94317b 100644 --- a/gcc/config/sh/sh-protos.h +++ b/gcc/config/sh/sh-protos.h @@ -162,7 +162,7 @@ extern bool sh_expand_t_scc (rtx *); extern rtx sh_gen_truncate (enum machine_mode, rtx, int); extern bool sh_vector_mode_supported_p (enum machine_mode); extern bool sh_cfun_trap_exit_p (void); -extern rtx sh_find_equiv_gbr_addr (rtx cur_insn, rtx mem); +extern rtx sh_find_equiv_gbr_addr (rtx_insn* cur_insn, rtx mem); extern int sh_eval_treg_value (rtx op); extern HOST_WIDE_INT sh_disp_addr_displacement (rtx mem_op); extern int sh_max_mov_insn_displacement (machine_mode mode, bool consider_sh2a); diff --git a/gcc/config/sh/sh.c b/gcc/config/sh/sh.c index 07ed73ea47a..fdf61de3c63 100644 --- a/gcc/config/sh/sh.c +++ b/gcc/config/sh/sh.c @@ -13421,11 +13421,10 @@ base_reg_disp::disp (void) const } /* Find the base register and calculate the displacement for a given - address rtx 'x'. - This is done by walking the insn list backwards and following SET insns - that set the value of the specified reg 'x'. */ + address rtx 'x'. */ static base_reg_disp -sh_find_base_reg_disp (rtx insn, rtx x, disp_t disp = 0, rtx base_reg = NULL) +sh_find_base_reg_disp (rtx_insn* insn, rtx x, disp_t disp = 0, + rtx base_reg = NULL) { if (REG_P (x)) { @@ -13439,31 +13438,37 @@ sh_find_base_reg_disp (rtx insn, rtx x, disp_t disp = 0, rtx base_reg = NULL) if (REGNO (x) < FIRST_PSEUDO_REGISTER) return base_reg_disp (base_reg != NULL ? base_reg : x, disp); - /* Try to find the previous insn that sets the reg. */ - for (rtx i = prev_nonnote_insn (insn); i != NULL; - i = prev_nonnote_insn (i)) + /* Find the def of the reg and trace it. If there are more than one + defs and they are not the same, assume it's not safe to proceed. */ + rtx_insn* last_i = NULL; + rtx last_set = NULL; + for (df_ref d = DF_REG_DEF_CHAIN (REGNO (x)); d != NULL; + d = DF_REF_NEXT_REG (d)) { - if (REGNO_REG_SET_P (regs_invalidated_by_call_regset, GBR_REG) - && CALL_P (i)) - break; - - if (!NONJUMP_INSN_P (i)) - continue; + rtx set = const_cast (set_of (x, DF_REF_INSN (d))); - rtx p = PATTERN (i); - if (p != NULL && GET_CODE (p) == SET && REG_P (XEXP (p, 0)) - && REGNO (XEXP (p, 0)) == REGNO (x)) + /* Accept multiple defs, as long as they are equal. */ + if (last_set == NULL || rtx_equal_p (last_set, set)) + { + last_i = DF_REF_INSN (d); + last_set = set; + } + else { - /* If the recursion can't find out any more details about the - source of the set, then this reg becomes our new base reg. */ - return sh_find_base_reg_disp (i, XEXP (p, 1), disp, XEXP (p, 0)); + last_i = NULL; + last_set = NULL; + break; } } - /* When here, no previous insn was found that sets the reg. - The input reg is already the base reg. */ - return base_reg_disp (x, disp); - } + if (last_set != NULL && last_i != NULL) + return sh_find_base_reg_disp (last_i, XEXP (last_set, 1), disp, + XEXP (last_set, 0)); + + /* When here, no previous insn was found that sets the reg. + The input reg is already the base reg. */ + return base_reg_disp (x, disp); + } else if (GET_CODE (x) == PLUS) { @@ -13493,19 +13498,47 @@ sh_find_base_reg_disp (rtx insn, rtx x, disp_t disp = 0, rtx base_reg = NULL) based memory address and return the corresponding new memory address. Return NULL_RTX if not found. */ rtx -sh_find_equiv_gbr_addr (rtx insn, rtx mem) +sh_find_equiv_gbr_addr (rtx_insn* insn, rtx mem) { - if (!MEM_P (mem)) + if (!MEM_P (mem) || gbr_address_mem (mem, GET_MODE (mem))) return NULL_RTX; /* Leave post/pre inc/dec or any other side effect addresses alone. */ if (side_effects_p (XEXP (mem, 0))) return NULL_RTX; + /* When not optimizing there might be no dataflow available. */ + if (df == NULL) + return NULL_RTX; + base_reg_disp gbr_disp = sh_find_base_reg_disp (insn, XEXP (mem, 0)); if (gbr_disp.is_reg () && REGNO (gbr_disp.reg ()) == GBR_REG) { + /* If GBR is marked as call clobbered we bail out if we see a call. + FIXME: Actually should check if this mem refers to the gbr value + before or after the call. If there is a store_gbr preceeding this + mem, it's safe to use GBR for this mem. + + If GBR is not marked as call clobbered, but there is some other + def than a call, it's probably a load_gbr upon which we also + bail out to be on the safe side. + FIXME: Should check if we have a use-after-def case, such as + the call case above. */ + for (df_ref d = DF_REG_DEF_CHAIN (GBR_REG); d != NULL; + d = DF_REF_NEXT_REG (d)) + { + if (CALL_P (DF_REF_INSN (d))) + { + if (REGNO_REG_SET_P (regs_invalidated_by_call_regset, GBR_REG)) + return NULL_RTX; + else + continue; + } + else + return NULL_RTX; + } + rtx disp = GEN_INT (gbr_disp.disp ()); if (gbr_displacement (disp, GET_MODE (mem))) return gen_rtx_PLUS (SImode, gen_rtx_REG (SImode, GBR_REG), disp); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 6a63e33ab93..f34d55f8a75 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2014-10-12 Oleg Endo + + PR target/59401 + PR target/54760 + * gcc.target/pr54760-5.c: New. + * gcc.target/pr54760-6.c: New. + * gcc.target/sh/pr59401-1.c: New. + 2014-10-11 Francois-Xavier Coudert PR fortran/48979 diff --git a/gcc/testsuite/gcc.target/sh/pr54760-5.c b/gcc/testsuite/gcc.target/sh/pr54760-5.c new file mode 100644 index 00000000000..30dfd314e75 --- /dev/null +++ b/gcc/testsuite/gcc.target/sh/pr54760-5.c @@ -0,0 +1,26 @@ +/* Check that the GBR address optimization works when there are multiple + GBR register definitions and function calls, if the GBR is marked as a + call saved register. */ +/* { dg-do compile } */ +/* { dg-options "-O1 -fcall-saved-gbr" } */ +/* { dg-skip-if "" { "sh*-*-*" } { "-m5*"} { "" } } */ +/* { dg-final { scan-assembler-not "stc\tgbr" } } */ + +typedef struct +{ + int x, y, z, w; +} tcb_t; + +extern void test_00 (void); + +int +test_01 (int x, volatile int* y, int a) +{ + if (a) + test_00 (); + + y[0] = 1; + + tcb_t* tcb = (tcb_t*)__builtin_thread_pointer (); + return (a & 5) ? tcb->x : tcb->w; +} diff --git a/gcc/testsuite/gcc.target/sh/pr54760-6.c b/gcc/testsuite/gcc.target/sh/pr54760-6.c new file mode 100644 index 00000000000..4bf94c32ec6 --- /dev/null +++ b/gcc/testsuite/gcc.target/sh/pr54760-6.c @@ -0,0 +1,19 @@ +/* Check that the GBR address optimization works when the GBR register + definition is not in the same basic block where the GBR memory accesses + are. */ +/* { dg-do compile } */ +/* { dg-options "-O1" } */ +/* { dg-skip-if "" { "sh*-*-*" } { "-m5*"} { "" } } */ +/* { dg-final { scan-assembler-not "stc\tgbr" } } */ + +typedef struct +{ + int x, y, z, w; +} tcb_t; + +int +test_00 (int a, tcb_t* b, int c) +{ + tcb_t* tcb = (tcb_t*)__builtin_thread_pointer (); + return (a & 5) ? tcb->x : tcb->w; +} diff --git a/gcc/testsuite/gcc.target/sh/pr59401-1.c b/gcc/testsuite/gcc.target/sh/pr59401-1.c new file mode 100644 index 00000000000..a2afe1ae6f7 --- /dev/null +++ b/gcc/testsuite/gcc.target/sh/pr59401-1.c @@ -0,0 +1,20 @@ +/* Check that the GBR address optimization does not produce wrong memory + accesses. In this case the GBR value must be stored to a normal register + and a GBR memory access must not be done. */ +/* { dg-do compile } */ +/* { dg-options "-O1" } */ +/* { dg-skip-if "" { "sh*-*-*" } { "-m5*"} { "" } } */ +/* { dg-final { scan-assembler "stc\tgbr" } } */ +/* { dg-final { scan-assembler "bf|bt" } } */ + +typedef struct +{ + int x, y, z, w; +} tcb_t; + +int +test_00 (int a, tcb_t* b) +{ + tcb_t* tcb = (a & 5) ? (tcb_t*)__builtin_thread_pointer () : b; + return tcb->w + tcb->x; +} -- 2.30.2