On 6/20/20 5:18 AM, Zhongyunde wrote:
> In some target, it is limited to issue two insns with change the same 
> register.(The insn 73 start with insn:TI, so it will be issued together with 
> others insns  until a new insn start with insn:TI, such as insn 71)
> The regrename can known the mode V2VF in insn 73 need two successive 
> registers, i.e. v2 and v3, here is dump snippet before the regrename.
>
> (insn:TI 73 76 71 4 (set (reg/v:V2VF 37 v2 [orig:180 _62 ] [180])
>         (unspec:V2VF [
>                 (reg/v:VHF 43 v8 [orig:210 Dest_value ] [210])
>                 (reg/v:VHF 43 v8 [orig:210 Dest_value ] [210])
>             ] UNSPEC_HFSQMAG_32X32)) "../test_modify.c":57 710 {hfsqmag_v2vf}
>      (expr_list:REG_DEAD (reg/v:VHF 43 v8 [orig:210 Dest_value ] [210])
>         (expr_list:REG_UNUSED (reg:VHF 38 v3)
>             (expr_list:REG_STAGE (const_int 2 [0x2])
>                 (expr_list:REG_CYCLE (const_int 2 [0x2])
>                     (expr_list:REG_UNITS (const_int 256 [0x100])
>                         (nil)))))))
>
> (insn 71 73 243 4 (set (reg:VHF 43 v8 [orig:265 MEM[(const vfloat32x16 
> *)Src_base_134] ] [265])
>         (mem:VHF (reg/v/f:DI 13 a13 [orig:207 Src_base ] [207]) [1 MEM[(const 
> vfloat32x16 *)Src_base_134]+0 S64 A512])) "../test_modify.c":56 450 
> {movvhf_internal}
>      (expr_list:REG_STAGE (const_int 1 [0x1])
>         (expr_list:REG_CYCLE (const_int 2 [0x2])
>             (nil))))
>
> Then, in the regrename, the insn 71 will be transformed into following code 
> with register v3, so there is an conflict between insn 73 and insn 71, as 
> both of them set the v3 register.
>
> Register v2 (2): 73 [SVEC_REGS]
> Register v8 (1): 71 [VEC_ALL_REGS]
>
> ....
>
> (insn 71 73 243 4 (set (reg:VHF 38 v3 [orig:265 MEM[(const vfloat32x16 
> *)Src_base_134] ] [265])
>         (mem:VHF (reg/v/f:DI 13 a13 [orig:207 Src_base ] [207]) [1 MEM[(const 
> vfloat32x16 *)Src_base_134]+0 S64 A512])) "../test_modify.c":56 450 
> {movvhf_internal}
>      (expr_list:REG_STAGE (const_int 1 [0x1])
>         (expr_list:REG_CYCLE (const_int 2 [0x2])
>
> 2644.diff
>
> diff --git a/gcc/regrename.c b/gcc/regrename.c
> index c38173a77..8eea34dd8 100644
> --- a/gcc/regrename.c
> +++ b/gcc/regrename.c
> @@ -284,6 +284,48 @@ create_new_chain (unsigned this_regno, unsigned 
> this_nregs, rtx *loc,
>    return head;
>  }
>  
> +/* For a def-use chain HEAD, find which registers with REG_UNUSED
> +   conflict its VLIW constraint and set the corresponding bits in *PSET.  */
> +
> +static void
> +merge_vliw_overlapping_regs (HARD_REG_SET *pset, struct du_head *head)
> +{
> +  rtx_insn *insn;
> +  rtx_insn *cur_insn = head->first->insn;
> +  basic_block bb = BLOCK_FOR_INSN (cur_insn);
> +
> +  /* Only SMS related basic block may generate VLIW before regrename pass.  
> */
> +  if ((bb->flags & BB_DISABLE_SCHEDULE) == 0 || (bb->flags & BB_MODIFIED) != 
> 0)
> +    return;
Presumably this is to keep the cost down.  But ISTM that it might be
possible for a target to get TImode set via a peep2.  Similarly if a
plugin hooks into the pass manager to insert its own passes.  And it may
be the case that we could add other passes that might set TImode on the
insn in the future.  So I don't think this is necessarily a good test.

Closely related, I think we insert the TImode insn markers independent
of the target in the selective scheduler.  So your patch could pessimize
other targets that use the selective scheduler.



So, I think the target files need to indicate they need to constrain
regrename's behavior.  This probably means a target hook that your
target will need to set.   You could then test that target hook rather
than looking at the bb flags.

 

> b
> +
> +  for (insn = prev_active_insn (cur_insn); insn && BLOCK_FOR_INSN (insn) == 
> bb;
> +       insn = prev_active_insn (insn))
> +    {
> +      rtx note;
> +
> +      for (note = REG_NOTES (insn); note; note = XEXP (note, 1))
> +        {
> +          if (REG_NOTE_KIND (note) == REG_UNUSED)
So does this only happen when there's a REG_UNUSED note?  That's useful
to know.


> +          {
> +            int regno = REGNO (XEXP (note, 0));
> +            bool old = TEST_HARD_REG_BIT(*pset, regno);
> +            SET_HARD_REG_BIT (*pset, nregs);
So can't XEXP (note, 0) have a mode which would cover multiple
registers?  And if so, don't we need to iterate over all the covered
registers?


Jeff

Reply via email to