On 10/1/18 7:45 AM, H.J. Lu wrote:
> You may have undone:
> 
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=218059

Yes, the code above also needed to be modified to handle conflicts being
added at definitions rather than at uses.  The patch below does that.
I don't really have access to a i686 (ie, 32-bit) system to test on and
I'm not sure how to force the test to be run in 32-bit mode on a 64-bit
build, but it does fix the assembler for the pr63534.c test case.

That said, looking at the rtl for the test case, I see the following
before RA:

(insn 5 2 6 2 (set (reg:SI 3 bx)
        (reg:SI 82)) "pr63534.c":10 85 {*movsi_internal}
     (nil))
(call_insn 6 5 7 2 (call (mem:QI (symbol_ref:SI ("bar") [flags 0x41]  
<function_decl 0x7f30c7548000 bar>) [0 barD.1498 S1 A8])
        (const_int 0 [0])) "pr63534.c":10 687 {*call}
     (expr_list:REG_DEAD (reg:SI 3 bx)
        (expr_list:REG_CALL_DECL (symbol_ref:SI ("bar") [flags 0x41]  
<function_decl 0x7f30c7548000 bar>)
            (nil)))
    (expr_list (use (reg:SI 3 bx))
        (nil)))
(insn 7 6 8 2 (set (reg:SI 3 bx)
        (reg:SI 82)) "pr63534.c":11 85 {*movsi_internal}
     (expr_list:REG_DEAD (reg:SI 82)
        (nil)))
(call_insn 8 7 0 2 (call (mem:QI (symbol_ref:SI ("bar") [flags 0x41]  
<function_decl 0x7f30c7548000 bar>) [0 barD.1498 S1 A8])
        (const_int 0 [0])) "pr63534.c":11 687 {*call}
     (expr_list:REG_DEAD (reg:SI 3 bx)
        (expr_list:REG_CALL_DECL (symbol_ref:SI ("bar") [flags 0x41]  
<function_decl 0x7f30c7548000 bar>)
            (nil)))
    (expr_list (use (reg:SI 3 bx))
        (nil)))

Now that we handle conflicts at definitions and the pic hard reg
is set via a copy from the pic pseudo, my PATCH 2 is setup to
handle exactly this scenario (ie, a copy between a pseudo and
a hard reg).  I looked at the asm output from a build with both
PATCH 1 and PATCH 2, and yes, it also does not add the conflict
between the pic pseudo and pic hard reg, so our other option to
fix PR87479 is to apply PATCH 2.  However, since PATCH 2 handles
the pic pseudo and pic hard reg conflict itself, that means we
don't need the special pic conflict code and it can be removed!
I'm going to update PATCH 2 to remove that pic handling code
and send it through bootstrap and regtesting.

H.J., can you confirm that the following patch not only fixes
the bug you opened, but also doesn't introduce any more?
Once I've updated PATCH 2, I'd like you to test/bless that
one as well.  Thanks.

Peter


gcc/
        PR rtl-optimization/87479
        * ira-lives.c (process_bb_node_lives): Move handling of pic pseudo
        and pic hard reg conflict to the insn that sets pic hard reg.
        * lra-lives.c (mark_regno_dead) <check_pic_pseudo_p>: New function
        argument.  Use it.
        (process_bb_lives): Use new argument to mark_regno_dead.
        Don't handle pic pseudo and pic hard reg conflict when processing
        function call arguments.

Index: gcc/ira-lives.c
===================================================================
--- gcc/ira-lives.c     (revision 264758)
+++ gcc/ira-lives.c     (working copy)
@@ -1108,20 +1108,25 @@ process_bb_node_lives (ira_loop_tree_node_t loop_t
 
          call_p = CALL_P (insn);
 #ifdef REAL_PIC_OFFSET_TABLE_REGNUM
-         int regno;
+         unsigned int regno;
+         rtx set;
          bool clear_pic_use_conflict_p = false;
-         /* Processing insn usage in call insn can create conflict
-            with pic pseudo and pic hard reg and that is wrong.
-            Check this situation and fix it at the end of the insn
-            processing.  */
-         if (call_p && pic_offset_table_rtx != NULL_RTX
+         /* Processing insn definition of REAL_PIC_OFFSET_TABLE_REGNUM
+            can create a conflict between the pic pseudo and pic hard reg
+            and that is wrong.  Check this situation and fix it at the end
+            of the insn processing.  */
+         if (pic_offset_table_rtx != NULL_RTX
              && (regno = REGNO (pic_offset_table_rtx)) >= FIRST_PSEUDO_REGISTER
-             && (a = ira_curr_regno_allocno_map[regno]) != NULL)
+             && (a = ira_curr_regno_allocno_map[regno]) != NULL
+             && (set = single_set (insn)) != NULL_RTX
+             && REG_P (SET_DEST (set))
+             && REGNO (SET_DEST (set)) == REAL_PIC_OFFSET_TABLE_REGNUM
+             && REG_P (SET_SRC (set))
+             && REGNO (SET_SRC (set)) == regno)
            clear_pic_use_conflict_p
-               = (find_regno_fusage (insn, USE, REAL_PIC_OFFSET_TABLE_REGNUM)
-                  && ! TEST_HARD_REG_BIT (OBJECT_CONFLICT_HARD_REGS
-                                          (ALLOCNO_OBJECT (a, 0)),
-                                          REAL_PIC_OFFSET_TABLE_REGNUM));
+               = ! TEST_HARD_REG_BIT (OBJECT_CONFLICT_HARD_REGS
+                                      (ALLOCNO_OBJECT (a, 0)),
+                                      REAL_PIC_OFFSET_TABLE_REGNUM);
 #endif
 
          /* Mark each defined value as live.  We need to do this for
Index: gcc/lra-lives.c
===================================================================
--- gcc/lra-lives.c     (revision 264758)
+++ gcc/lra-lives.c     (working copy)
@@ -342,7 +342,8 @@ mark_regno_live (int regno, machine_mode mode, int
    BB_GEN_PSEUDOS and BB_KILLED_PSEUDOS.  Return TRUE if the liveness
    tracking sets were modified, or FALSE if nothing changed.  */
 static bool
-mark_regno_dead (int regno, machine_mode mode, int point)
+mark_regno_dead (int regno, machine_mode mode, int point,
+                bool check_pic_pseudo_p)
 {
   int last;
   bool changed = false;
@@ -350,7 +351,7 @@ static bool
   if (regno < FIRST_PSEUDO_REGISTER)
     {
       for (last = end_hard_regno (mode, regno); regno < last; regno++)
-       make_hard_regno_dead (regno, false);
+       make_hard_regno_dead (regno, check_pic_pseudo_p);
     }
   else
     {
@@ -851,9 +852,11 @@ process_bb_lives (basic_block bb, int &curr_point,
       for (reg = curr_id->regs; reg != NULL; reg = reg->next)
        if (reg->type == OP_OUT
            && ! reg_early_clobber_p (reg, n_alt) && ! reg->subreg_p)
+         /* Don't create a conflict between REAL_PIC_OFFSET_TABLE_REGNUM
+            and the pic pseudo.  */
          need_curr_point_incr
            |= mark_regno_dead (reg->regno, reg->biggest_mode,
-                               curr_point);
+                               curr_point, true);
 
       for (reg = curr_static_id->hard_regs; reg != NULL; reg = reg->next)
        if (reg->type == OP_OUT
@@ -863,9 +866,7 @@ process_bb_lives (basic_block bb, int &curr_point,
       if (curr_id->arg_hard_regs != NULL)
        for (i = 0; (regno = curr_id->arg_hard_regs[i]) >= 0; i++)
          if (regno >= FIRST_PSEUDO_REGISTER)
-           /* It is a clobber.  Don't create conflict of used
-              REAL_PIC_OFFSET_TABLE_REGNUM and the pic pseudo.  */
-           make_hard_regno_dead (regno - FIRST_PSEUDO_REGISTER, true);
+           make_hard_regno_dead (regno - FIRST_PSEUDO_REGISTER, false);
 
       if (call_p)
        {
@@ -939,7 +940,7 @@ process_bb_lives (basic_block bb, int &curr_point,
            && reg_early_clobber_p (reg, n_alt) && ! reg->subreg_p)
          need_curr_point_incr
            |= mark_regno_dead (reg->regno, reg->biggest_mode,
-                               curr_point);
+                               curr_point, false);
 
       for (reg = curr_static_id->hard_regs; reg != NULL; reg = reg->next)
        if (reg->type == OP_OUT


Reply via email to