On Mon, Oct 29, 2012 at 5:11 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Mon, Oct 29, 2012 at 4:41 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
>> On Mon, Oct 29, 2012 at 9:38 AM, Vladimir Makarov <vmaka...@redhat.com>
>> wrote:
>>> On 12-10-29 12:21 PM, Richard Sandiford wrote:
>>>>
>>>> Vladimir Makarov <vmaka...@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.
--
H.J.
gcc/
2012-10-29 H.J. Lu <hongjiu...@intel.com>
PR middle-end/55116
* rtlanal.c (strip_address_mutations): Handle SUBREG only for
Pmode.
gcc/testsuite/
2012-10-29 H.J. Lu <hongjiu...@intel.com>
PR middle-end/55116
* gcc.target/i386/pr55116-2.c: New file.
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);
+}
