This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 2/6] Andes nds32: machine description of nds32 porting (1).
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Chung-Ju Wu <jasonwucj at gmail dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Sun, 06 Oct 2013 10:36:52 +0100
- Subject: Re: [PATCH 2/6] Andes nds32: machine description of nds32 porting (1).
- Authentication-results: sourceware.org; auth=none
- References: <CADj25HMFJUOHWC5ZvDOrJH+xkCoN8sxHXhD2vb36bM95irWEdg at mail dot gmail dot com> <Pine dot LNX dot 4 dot 64 dot 1307092307170 dot 29921 at digraph dot polyomino dot org dot uk> <51EFF75C dot 2070705 at gmail dot com> <Pine dot LNX dot 4 dot 64 dot 1307241606370 dot 31023 at digraph dot polyomino dot org dot uk> <51F0F15E dot 1050606 at gmail dot com> <522CA211 dot 60200 at gmail dot com> <8761u4losy dot fsf at talisman dot default> <5245CAB9 dot 8050400 at gmail dot com> <87siwnlvi5 dot fsf at talisman dot default> <52505830 dot 9080800 at gmail dot com>
Thanks for the updates.
Chung-Ju Wu <jasonwucj@gmail.com> writes:
> On 9/29/13 7:25 PM, Richard Sandiford wrote:
>> Chung-Ju Wu <jasonwucj@gmail.com> writes:
>>
>>> + /* We need to provide a customized rtx which contains
>>> + necessary information for data analysis,
>>> + so we create a parallel rtx like this:
>>> + (parallel [(unspec [(reg: Rb)
>>> + (reg: Re)
>>> + (const_int En4)] UNSPEC_STACK_PUSH_MULTIPLE)
>>> + (use (reg:SI Rb))
>>> + (use (reg:SI Re))
>>> + (set (mem (plus (reg:SI SP_REGNUM) (const_int -4)))
>>> + (reg:SI LP_REGNUM))
>>> + (set (mem (plus (reg:SI SP_REGNUM) (const_int -8)))
>>> + (reg:SI GP_REGNUM))
>>> + (set (mem (plus (reg:SI SP_REGNUM) (const_int -12)))
>>> + (reg:SI FP_REGNUM))
>>> + (set (mem (plus (reg:SI SP_REGNUM) (const_int -16)))
>>> + (reg:SI Re))
>>> + ...
>>> + (set (mem (plus (reg:SI SP_REGNUM) (const_int -28)))
>>> + (reg:SI Rb+1))
>>> + (set (mem (plus (reg:SI SP_REGNUM) (const_int -32)))
>>> + (reg:SI Rb))
>>> + (set (reg:SI SP_REGNUM)
>>> + (plus (reg:SI SP_REGNUM) (const_int -32)))]) */
>>
>> The "use"s here should be redundant, since the "set"s say that the
>> registers are used. Same comment for:
>>
>>> + /* We need to provide a customized rtx which contains
>>> + necessary information for data analysis,
>>> + so we create a parallel rtx like this:
>>> + (NOTE: We have to clearly claim that we are using/clobbering Rb and Re,
>>> + otherwise it may be renamed by optimization.)
>>> + (parallel [(unspec [(reg: Rb)
>>> + (reg: Re)
>>> + (const_int En4)] UNSPEC_STACK_POP_MULTIPLE)
>>> + (use (reg:SI Rb))
>>> + (use (reg:SI Re))
>>> + (clobber (reg:SI Rb))
>>> + (clobber (reg:SI Re))
>>> + (set (reg:SI Rb)
>>> + (mem (plus (reg:SI SP_REGNUM) (const_int 0))))
>>> + (set (reg:SI Rb+1)
>>> + (mem (plus (reg:SI SP_REGNUM) (const_int 4))))
>>> + ...
>>> + (set (reg:SI Re)
>>> + (mem (plus (reg:SI SP_REGNUM) (const_int 16))))
>>> + (set (reg:SI FP_REGNUM)
>>> + (mem (plus (reg:SI SP_REGNUM) (const_int 20))))
>>> + (set (reg:SI GP_REGNUM)
>>> + (mem (plus (reg:SI SP_REGNUM) (const_int 24))))
>>> + (set (reg:SI LP_REGNUM)
>>> + (mem (plus (reg:SI SP_REGNUM) (const_int 28))))
>>> + (set (reg:SI SP_REGNUM)
>>> + (plus (reg:SI SP_REGNUM) (const_int 32)))]) */
>>
>> the "use"s and "clobber"s here. As far as the note goes, adding clobbers
>> isn't guaranteed to stop renaming, and it's not really valid for an rtx
>> to assign to the same register twice.
>>
>> The best way of preventing renaming is to have a match_parallel that
>> checks every "set", a bit like you already did for the load/store multiple.
>> That should make the "unspec" unnecessary too.
>>
>
> Now we remove all "use"s and "clobber"s from parallel rtx and
> use predicate function to check stack push/pop operation.
> Furthermore, once I remove unspec rtx as you suggested,
> I notice gcc is aware of the def-use dependency and it wouldn't
> perform unexpected register renaming. So I think it was my
> faulty design of placing unspec/use/clobber within parallel rtx.
FWIW, it'd probably be better for nds32_valid_stack_push_pop to check
all the SETs in the PARALLEL, like nds32_valid_multiple_load_store does.
They could share a subroutine that checks for conesecutive loads and stores.
I.e. something like:
static bool
nds32_valid_multiple_load_store_1 (rtx op, bool load_p, int start, int end)
{
...
}
bool
nds32_valid_multiple_load_store (rtx op, bool load_p)
{
return nds32_valid_multiple_load_store_1 (op, load_p, 0, XVECLEN (op, 0));
}
bool
nds32_valid_multiple_push_pop (rtx op, bool load_p)
{
...handle $lp, $gp and $fp cases, based on cframe->...
if (!nds32_valid_multiple_load_store_1 (op, load_p, i, count - 1))
return false;
...check last element...
}
Thanks,
Richard