diff mbox

RFA: patch to fix PR55116

Message ID CAMe9rOpmegrsn5Rxj9t8RuHa9OQy5jQSPUxY=AzbpiaAXQdnSA@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu Oct. 30, 2012, 6:02 a.m. UTC 
On Mon, Oct 29, 2012 at 5:11 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Oct 29, 2012 at 4:41 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, Oct 29, 2012 at 9:38 AM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>>> On 12-10-29 12:21 PM, Richard Sandiford wrote:
>>>>
>>>> Vladimir Makarov <vmakarov@redhat.com> writes:
>>>>>
>>>>>     H.J. in
>>>>>
>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55116
>>>>>
>>>>>     reported an interesting address
>>>>>
>>>>> (and:DI (subreg:DI (plus:SI (ashift:SI (reg:SI 96 [ glob_vol_int.22 ])
>>>>>                   (const_int 2 [0x2]))
>>>>>               (symbol_ref:SI ("glob_vol_int_arr") <var_decl
>>>>> 0x7ffff03c2720 glob_vol_int_arr>)) 0)
>>>>>       (const_int 4294967295 [0xffffffff]))
>>>>>
>>>>>     which can not be correctly extracted.  Here `and' with `subreg'
>>>>> behaves as an address mutation.
>>>>>
>>>>>     The following patch fixes the problem.
>>>>>
>>>>> Ok to commit, Richard?
>>>>
>>>> Heh, I wondered if subregs might still be used like that, and was almost
>>>> tempted to add them just in case.
>>>>
>>>> I think this particular case is really a failed canonicalisation and that:
>>>>
>>>>     (and:DI (subreg:DI (foo:SI ...) 0) (const_int 0xffffffff))
>>>>
>>>> ought to be:
>>>>
>>>>     (zero_extend:DI (foo:SI ...))
>>>
>>> Yes, that was my thought too.
>>>
>>>> But I know I've approved MIPS patches to accept
>>>> (and:DI ... (const_int 0xffffffff)) as an alternative.
>>>>
>>>>> Index: rtlanal.c
>>>>> ===================================================================
>>>>> --- rtlanal.c   (revision 192942)
>>>>> +++ rtlanal.c   (working copy)
>>>>> @@ -5459,6 +5459,11 @@ strip_address_mutations (rtx *loc, enum
>>>>>          else if (code == AND && CONST_INT_P (XEXP (*loc, 1)))
>>>>>           /* (and ... (const_int -X)) is used to align to X bytes.  */
>>>>>           loc = &XEXP (*loc, 0);
>>>>> +      else if (code == SUBREG
>>>>> +              && ! REG_P (XEXP (*loc, 0)) && ! MEM_P (XEXP (*loc, 0)))
>>>>> +       /* (subreg (operator ...) ...) usually inside and is used for
>>>>> +          mode conversion too.  */
>>>>> +       loc = &XEXP (*loc, 0);
>>>>
>>>> I think the condition should be:
>>>>
>>>>        else if (code == SUBREG
>>>>                 && !OBJECT_P (SUBREG_REG (*loc))
>>>>                 && subreg_lowpart (*loc))
>>>>
>>>> OK with that change, if it works.
>>>>
>>> Yes, it works.
>>> I've submitted the following patch.
>>>
>>
>> It doesn't work right.  I will create a new testcase.
>>
>
>

This patch limits SUBREG to Pmode.  Tested on Linux/x86-64.
OK to install?

Thanks.
diff mbox

Patch

diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 43d4cb8..d076ad6 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -5460,6 +5460,7 @@  strip_address_mutations (rtx *loc, enum rtx_code
*outer_code)
 	/* (and ... (const_int -X)) is used to align to X bytes.  */
 	loc = &XEXP (*loc, 0);
       else if (code == SUBREG
+	       && GET_MODE (*loc) == Pmode
                && !OBJECT_P (SUBREG_REG (*loc))
                && subreg_lowpart_p (*loc))
 	/* (subreg (operator ...) ...) inside and is used for mode
diff --git a/gcc/testsuite/gcc.target/i386/pr55116-2.c
b/gcc/testsuite/gcc.target/i386/pr55116-2.c
new file mode 100644
index 0000000..7ef8ead
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr55116-2.c
@@ -0,0 +1,86 @@ 
+/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-options "-O2 -mx32 -maddress-mode=long" } */
+
+typedef struct rtx_def *rtx;
+enum rtx_code { MINUS };
+union rtunion_def {
+  rtx rt_rtx;
+};
+typedef union rtunion_def rtunion;
+struct rtx_def {
+  enum rtx_code code: 16;
+  union u {
+    rtunion fld[1];
+  }
+  u;
+};
+rtx simplify_binary_operation (enum rtx_code code, int mode,
+			       rtx op0, rtx op1);
+struct simplify_plus_minus_op_data {
+  rtx op;
+  short neg;
+};
+void simplify_plus_minus (enum rtx_code code, int mode, rtx op0, rtx op1)
+{
+  struct simplify_plus_minus_op_data ops[8];
+  rtx tem = (rtx) 0;
+  int n_ops = 2, input_ops = 2;
+  int changed, canonicalized = 0;
+  int i, j;
+  __builtin_memset (ops, 0, sizeof (ops));
+  do
+    {
+      changed = 0;
+      for (i = 0; i < n_ops; i++)
+	{
+	  rtx this_op = ops[i].op;
+	  int this_neg = ops[i].neg;
+	  enum rtx_code this_code = ((enum rtx_code) (this_op)->code);
+	  switch (this_code)
+	    {
+	    case MINUS:
+	      if (n_ops == 7)
+		return;
+	      n_ops++;
+	      input_ops++;
+	      changed = 1;
+	      canonicalized |= this_neg;
+	      break;
+	    }
+	}
+    }
+  while (changed);
+  do
+    {
+      j =  n_ops - 1;
+      for (i = n_ops - 1; j >= 0; j--)
+	{
+	  rtx lhs = ops[j].op, rhs = ops[i].op;
+	  int lneg = ops[j].neg, rneg = ops[i].neg;
+	  if (lhs != 0 && rhs != 0)
+	    {
+	      enum rtx_code ncode = MINUS;
+	      if (((enum rtx_code) (lhs)->code) == MINUS)
+		tem = simplify_binary_operation (ncode, mode, lhs, rhs);
+	      if (tem && ! (((enum rtx_code) (tem)->code) == MINUS
+			    && ((((((tem)->u.fld[0]).rt_rtx))->u.fld[0]).rt_rtx) == lhs
+			    && ((((((tem)->u.fld[0]).rt_rtx))->u.fld[1]).rt_rtx) == rhs))
+		{
+		  lneg &= rneg;
+		  ops[i].op = tem;
+		  ops[i].neg = lneg;
+		  ops[j].op = (rtx) 0;
+		  changed = 1;
+		  canonicalized = 1;
+		}
+	    }
+	}
+      for (i = 0, j = 0; j < n_ops; j++)
+	if (ops[j].op)
+	  {
+	    ops[i] = ops[j];
+	    i++;
+	  }
+    }
+  while (changed);
+}