Patchwork PATCH: PR rtl-optimization/55093: [4.8 Regression] [x32] -maddress-mode=long failed

login
register
mail settings
Submitter H.J. Lu
Date Oct. 29, 2012, 1:06 p.m.
Message ID <20121029130624.GA18506@gmail.com>
Download mbox | patch
Permalink /patch/195002/
State New
Headers show

Comments

H.J. Lu - Oct. 29, 2012, 1:06 p.m.
Hi,

This patch changes get_elimination to check register number instead of
RTX.  Tested on Linux/x32 with -maddress-mode=long.  OK to install?

Thanks.


H.J.
---
gcc/

2012-10-29  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/55093
	* lra-eliminations.c (get_elimination): Check register number
	instead of RTX.

gcc/testsuite/

2012-10-29  H.J. Lu  <hongjiu.lu@intel.com>

	PR rtl-optimization/55093
	* gcc.target/i386/pr55093.c: New file.
Vladimir Makarov - Oct. 29, 2012, 2:47 p.m.
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.
Richard Sandiford - Oct. 29, 2012, 3:15 p.m.
"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 < &reg_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
H.J. Lu - Oct. 29, 2012, 10:44 p.m.
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 < &reg_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.
H.J. Lu - Oct. 29, 2012, 10:49 p.m.
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 < &reg_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.

Patch

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;
+}