| Message ID | 20121029130624.GA18506@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 12-10-29 9:06 AM, H.J. Lu wrote: > Hi, > > This patch changes get_elimination to check register number instead of > RTX. Tested on Linux/x32 with -maddress-mode=long. OK to install? > > Yes. Thanks, H.J.
"H.J. Lu" <hjl.tools@gmail.com> writes: > Hi, > > This patch changes get_elimination to check register number instead of > RTX. Tested on Linux/x32 with -maddress-mode=long. OK to install? FWIW, this doesn't sound right to me, at least not without more justification. The idea is that things like frame_pointer_rtx are supposed to be unique, so the original code: > if ((ep = elimination_map[hard_regno]) != NULL) > - return ep->from_rtx != reg ? NULL : ep; > from != hard_regno ? NULL : ep; ought to be correct in itself. reload did the same thing: for (ep = reg_eliminate; ep < ®_eliminate[NUM_ELIMINABLE_REGS]; ep++) if (ep->from_rtx == x && ep->can_eliminate) return plus_constant (Pmode, ep->to_rtx, ep->previous_offset); It sounds on the face of it like the bug is elsewhere. Richard
On Mon, Oct 29, 2012 at 8:15 AM, Richard Sandiford <rdsandiford@googlemail.com> wrote: > "H.J. Lu" <hjl.tools@gmail.com> writes: >> Hi, >> >> This patch changes get_elimination to check register number instead of >> RTX. Tested on Linux/x32 with -maddress-mode=long. OK to install? > > FWIW, this doesn't sound right to me, at least not without more justification. > The idea is that things like frame_pointer_rtx are supposed to be unique, > so the original code: > >> if ((ep = elimination_map[hard_regno]) != NULL) >> - return ep->from_rtx != reg ? NULL : ep; >> from != hard_regno ? NULL : ep; > > ought to be correct in itself. reload did the same thing: > > for (ep = reg_eliminate; ep < ®_eliminate[NUM_ELIMINABLE_REGS]; > ep++) > if (ep->from_rtx == x && ep->can_eliminate) > return plus_constant (Pmode, ep->to_rtx, ep->previous_offset); > > It sounds on the face of it like the bug is elsewhere. > LRA has if (REG_P (reg) && (ep = get_elimination (reg)) != NULL) { rtx to_rtx = replace_p ? ep->to_rtx : ep->from_rtx; if (! replace_p) { offset += (ep->offset - ep->previous_offset); offset = trunc_int_for_mode (offset, GET_MODE (plus_cst_src)); } if (GET_CODE (XEXP (plus_cst_src, 0)) == SUBREG) to_rtx = gen_lowpart (GET_MODE (XEXP (plus_cst_src, 0)), to_rtx); Reload has rtx to_rtx = ep->to_rtx; offset += ep->offset; offset = trunc_int_for_mode (offset, GET_MODE (plus_cst_src)); if (GET_CODE (XEXP (plus_cst_src, 0)) == SUBREG) to_rtx = gen_lowpart (GET_MODE (XEXP (plus_cst_src, 0)), to_rtx); (gdb) call debug_rtx (ep->to_rtx) (reg/f:DI 7 sp) (gdb) call debug_rtx (ep->from_rtx) (reg/f:DI 16 argp) (gdb) gen_lowpart returns (reg/f:DI 7 sp) for reload and (reg:SI 16 argp) for LRA. They are caused by if (FRAME_POINTER_REGNUM != ARG_POINTER_REGNUM /* We should convert arg register in LRA after the elimination if it is possible. */ && xregno == ARG_POINTER_REGNUM && ! lra_in_progress) return -1; It doesn't work in this case.
On Mon, Oct 29, 2012 at 3:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Mon, Oct 29, 2012 at 8:15 AM, Richard Sandiford > <rdsandiford@googlemail.com> wrote: >> "H.J. Lu" <hjl.tools@gmail.com> writes: >>> Hi, >>> >>> This patch changes get_elimination to check register number instead of >>> RTX. Tested on Linux/x32 with -maddress-mode=long. OK to install? >> >> FWIW, this doesn't sound right to me, at least not without more justification. >> The idea is that things like frame_pointer_rtx are supposed to be unique, >> so the original code: >> >>> if ((ep = elimination_map[hard_regno]) != NULL) >>> - return ep->from_rtx != reg ? NULL : ep; >>> from != hard_regno ? NULL : ep; >> >> ought to be correct in itself. reload did the same thing: >> >> for (ep = reg_eliminate; ep < ®_eliminate[NUM_ELIMINABLE_REGS]; >> ep++) >> if (ep->from_rtx == x && ep->can_eliminate) >> return plus_constant (Pmode, ep->to_rtx, ep->previous_offset); >> >> It sounds on the face of it like the bug is elsewhere. >> > > LRA has > > if (REG_P (reg) && (ep = get_elimination (reg)) != NULL) > { > rtx to_rtx = replace_p ? ep->to_rtx : ep->from_rtx; > > if (! replace_p) > { > offset += (ep->offset - ep->previous_offset); > offset = trunc_int_for_mode (offset, GET_MODE (plus_cst_src)); > } > > if (GET_CODE (XEXP (plus_cst_src, 0)) == SUBREG) > to_rtx = gen_lowpart (GET_MODE (XEXP (plus_cst_src, 0)), to_rtx); > > Reload has > > rtx to_rtx = ep->to_rtx; > offset += ep->offset; > offset = trunc_int_for_mode (offset, GET_MODE (plus_cst_src)); > > if (GET_CODE (XEXP (plus_cst_src, 0)) == SUBREG) > to_rtx = gen_lowpart (GET_MODE (XEXP (plus_cst_src, 0)), > to_rtx); > > (gdb) call debug_rtx (ep->to_rtx) > (reg/f:DI 7 sp) > (gdb) call debug_rtx (ep->from_rtx) > (reg/f:DI 16 argp) > (gdb) > > gen_lowpart returns (reg/f:DI 7 sp) for reload and (reg:SI 16 argp) > for LRA. They are caused by > > if (FRAME_POINTER_REGNUM != ARG_POINTER_REGNUM > /* We should convert arg register in LRA after the elimination > if it is possible. */ > && xregno == ARG_POINTER_REGNUM > && ! lra_in_progress) > return -1; > > It doesn't work in this case. > This testcase shows that LRA can't convert arg register after the elimination.
diff --git a/gcc/lra-eliminations.c b/gcc/lra-eliminations.c index 2222d80..cbfbe7a 100644 --- a/gcc/lra-eliminations.c +++ b/gcc/lra-eliminations.c @@ -272,7 +272,7 @@ get_elimination (rtx reg) if ((hard_regno = REGNO (reg)) < 0 || hard_regno >= FIRST_PSEUDO_REGISTER) return NULL; if ((ep = elimination_map[hard_regno]) != NULL) - return ep->from_rtx != reg ? NULL : ep; + return ep->from != hard_regno ? NULL : ep; if ((offset = self_elim_offsets[hard_regno]) == 0) return NULL; /* This is an iteration to restore offsets just after HARD_REGNO diff --git a/gcc/testsuite/gcc.target/i386/pr55093.c b/gcc/testsuite/gcc.target/i386/pr55093.c new file mode 100644 index 0000000..76b4042 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr55093.c @@ -0,0 +1,80 @@ +/* { dg-do compile { target { ! { ia32 } } } } */ +/* { dg-options "-O2 -mx32 -maddress-mode=long" } */ + +typedef union tree_node *tree; +typedef const union tree_node *const_tree; +typedef struct { + unsigned long long low; + long long high; +} double_int; +struct real_value { +}; +struct real_format { + int has_signed_zero; +}; +extern const struct real_format * real_format_for_mode[]; +extern int real_isnegzero (const struct real_value *); +enum tree_code { REAL_CST, SSA_NAME }; +struct tree_base { + enum tree_code code : 16; + union { + unsigned int version; + } + u; +}; +extern void tree_check_failed (const_tree, const char *, int, const char *, ...) __attribute__ ((__noreturn__)); +union tree_node { + struct tree_base base; +}; +inline tree tree_check (tree __t, const char *__f, int __l, const char *__g, enum tree_code __c) { + if (((enum tree_code) (__t)->base.code) != __c) + tree_check_failed (__t, __f, __l, __g, __c, 0); + return __t; +} +struct prop_value_d { + int lattice_val; + tree value; + double_int mask; +}; +typedef struct prop_value_d prop_value_t; +static prop_value_t *const_val; +static void canonicalize_float_value (prop_value_t *); +typedef void (*ssa_prop_visit_stmt_fn) (prop_value_t); +typedef void (*ssa_prop_visit_phi_fn) (void); +typedef void (*ssa_prop_fold_stmt_fn) (void *gsi); +typedef void (*ssa_prop_get_value_fn) ( prop_value_t *val); +void ssa_propagate (ssa_prop_visit_stmt_fn, ssa_prop_visit_phi_fn); +int substitute_and_fold (ssa_prop_get_value_fn, ssa_prop_fold_stmt_fn); +void ccp_fold_stmt (void *); +static void get_constant_value (prop_value_t *val) { + canonicalize_float_value (val); +} +static void canonicalize_float_value (prop_value_t *val) { + int mode; + struct real_value d; + if (val->lattice_val != 1 + || ((enum tree_code) (val->value)->base.code) != REAL_CST) + return; + mode = val->lattice_val; + if (real_format_for_mode[mode]->has_signed_zero && real_isnegzero (&d)) + ccp_fold_stmt (0); +} +static void set_lattice_value (tree var, prop_value_t new_val) { + prop_value_t *old_val = &const_val[(tree_check ((var), "", + 0, "", + (SSA_NAME)))->base.u.version]; + canonicalize_float_value (&new_val); + canonicalize_float_value (old_val); +} +static void ccp_visit_phi_node (void) { + prop_value_t new_val; + set_lattice_value (0, new_val); +} +static void ccp_visit_stmt (prop_value_t v) { + set_lattice_value (0, v); +} +unsigned int do_ssa_ccp (void) { + ssa_propagate (ccp_visit_stmt, ccp_visit_phi_node); + substitute_and_fold (get_constant_value, ccp_fold_stmt); + return 0; +}