"H.J. Lu" <hjl.to...@gmail.com> writes:
> 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.
The address in this case is:
(plus:SI (mult:SI (reg/v:SI 223 [orig:154 j ] [154])
(const_int 8 [0x8]))
(subreg:SI (plus:DI (reg/f:DI 20 frame)
(const_int 32 [0x20])) 0))
which after Uros's subreg simplification patch shouldn't be allowed:
the subreg ought to be on the frame register rather than the plus.
The attached patch seems to work for the testcase. Does it work
more generally?
Richard
gcc/
* lra-eliminations.c (lra_eliminate_regs_1): Use simplify_gen_subreg
rather than gen_rtx_SUBREG.
Index: gcc/lra-eliminations.c
===================================================================
--- gcc/lra-eliminations.c (revision 192983)
+++ gcc/lra-eliminations.c (working copy)
@@ -550,7 +550,8 @@
return x;
}
else
- return gen_rtx_SUBREG (GET_MODE (x), new_rtx, SUBREG_BYTE (x));
+ return simplify_gen_subreg (GET_MODE (x), new_rtx,
+ GET_MODE (new_rtx), SUBREG_BYTE (x));
}
return x;
