Improve prepare_shrink_wrap to sink more instructions

On 11/09/14 21:39, Jeff Law wrote:
> 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",
> 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).

Jeff,

  thanks, I partially understand your meaning here.

   take the function "ira_implicitly_set_insn_hard_regs" in ira-lives.c for example,

   when generating address rtl, gcc will automatically generate "const" operator to prefix
   the address expression, like the following. so a simple CONSTANT_P check is enough in
   case there is no embedded register.

   (insn 309 310 308 3 (set (reg:DI 44 r15 [orig:94 ivtmp.674 ] [94])
         (const:DI (plus:DI (symbol_ref:DI ("recog_data") [flags 0x40]  <var_decl 0x2b2c2ff91510 recog_data>)
                 (const_int 480 [0x1e0])))) -1

   but for architecture like aarch64, the following instruction sequences to forming address
   may be generated

(insn 73 14 74 4 (set (reg/f:DI 20 x20 [99])
         (high:DI (symbol_ref:DI ("global_a") [flags 0xc0]  <var_decl 0x7ff755a60900 stats>))) 35 {*movdi_aarch64}
      (expr_list:REG_EQUIV (high:DI (symbol_ref:DI ("global_a") [flags 0xc0]  <var_decl 0x7ff755a60900 stats>))
         (nil)))

(insn 17 30 25 5 (set (reg/f:DI 4 x4 [83])
         (lo_sum:DI (reg/f:DI 20 x20 [99])
             (symbol_ref:DI ("global_a") [flags 0xc0]  <var_decl 0x7ff755a60900 stats>))) {add_losym_di}
      (expr_list:REG_EQUIV (symbol_ref:DI ("global_a") [flags 0xc0]  <var_decl 0x7ff755a60900 stats>)
         (nil)))

  while CONSTANT_P could not catch the latter lo_sum case, as the RTX_CLASS of lo_sum is RTX_OBJ not RTX_CONST_OBJ,
  so the

  the manually verification

       FOR_EACH_SUBRTX_VAR (iter, array, src, ALL)
         {
           rtx x = *iter;
           if (REG_P (x))
             {
               reg_num++;
               src_inner = x;
             }
           else if (!CONSTANT_P (x) && OBJECT_P (x))
             nonconstobj_num++;
         }

  is to catch cases like the lo_sum insn above which contains no more than 1 register and only contains
  RTX_CONST_OBJ.

> I suspect you still need the callback to verify the # of registers is
> just 1 so that the later tests work.  However, please don't use
> for_each_rtx, we're moving away from that to a more efficient walker
> FOR_EACH_SUBRTX.

   fixed, patch updated, please review.

   thanks.

   BTW, when I verifying this patch on gilbc x86-64 build, I found there may be another hiding
   issue when calculating live_in when splitting edge in move_insn_for_shrink_wrap, I will reply
   on that historical email thread directly.

Regards,
Jiong

>
> Jeff
>
>
>

Improve prepare_shrink_wrap to sink more instructions

Commit Message

Comments

Patch