This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Improve prepare_shrink_wrap to sink more instructions
- From: Wang Jiong <wong dot kwongyuan dot tools at gmail dot com>
- To: Jeff Law <law at redhat dot com>
- Cc: Jiong Wang <jiong dot wang at arm dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 11 Sep 2014 22:03:29 +0100
- Subject: Re: [PATCH] Improve prepare_shrink_wrap to sink more instructions
- Authentication-results: sourceware.org; auth=none
- References: <54087404 dot 1040608 at arm dot com> <540A137A dot 9010402 at redhat dot com> <540DB5CB dot 6080801 at arm dot com> <54120886 dot 30903 at redhat dot com>
2014-09-11 21:39 GMT+01:00 Jeff Law <law@redhat.com>:
>
> On 09/08/14 07:57, Jiong Wang wrote:
>>>
>>> Conceptually OK. Some questions/concerns about the implementation.
>>
>> Hi Jeff,
>>
>> thanks very much for your review.
>>>
>>> It seems to me that what you're trying to describe on the RHS is
>>> REG_P || CONSTANT_P
>>
>> yes.
>>
>> and actually I am trying to detect all rtx which contains any number
>> of RTX_CONST_OBJs and no more than one REG.
>>>
>>>
>>> Note that CONSTANT_P will catch things like (high (symbol_ref)) and
>>> (lo_sum (reg) (symbol_ref)) which are often used to build addresses on
>>> some targets.
>>>
>>> With that in mind, rather than using a for_each_rtx callback, test
>>> if (REG_P (src) || CONSTANT_P (src))
>>>
>>> Note that SRC, when it is a CONSTANT_P, may have a variety of forms
>>> with embedded registers. You'll need to verify that all of those
>>> registers are not assigned after the insn with the CONSTANT_P source
>>> operand. Right now you only perform that check when SRC is a REG_P.
>>
>>
>> I am using the for_each_rtx because I want to scan "src" so that
>> any sub-operands are checked, the number of REG and non-constant
>> objects are record in "reg_found" and "nonconst_found". the embedded
>> register found also record in the "reg" field of the structure
>> "rtx_search_arg",
>
Hi Jeff,
> Constants in this context are going to satisfy CONSTANT_P, you
don't need to manually verify them. It will include simple
constants and constants which are built up out of multiple
instructions (symbolic constants in particular).
===> thanks, will investigate this tomorrow.
>
> I suspect you still need the callback to verify the # of registers is just 1 so that the later tests work.
===> the reg_found will record the # of registers, and there will be
a check to make sure no register or just 1 register in the src.
===> so if "reg_found" is bigger than 1, then src will be assigned to
NULL_RTX, that we just stop the move.
+ if (arg.nonconst_found
+ || arg.reg_found > 1)
+ src = NULL_RTX;
===> am I missing something here?
>However, please don't use for_each_rtx, we're moving away from that to a more efficient walker FOR_EACH_SUBRTX.
===> OK, will fix. Thanks very much for review.
Regards,
Jiong
>
> Jeff