Hi Vlad,

First, I want to echo H-P's thanks for tackling this area.  I just wondered:

Vladimir Makarov <vmaka...@redhat.com> writes:
> The following patch is to solve PR48336, PR48342, PR48345.  The 
> profitable hard regs exclude hard regs which are prohibited for the 
> corresponding allocno mode. It is true for primary allocation and it is 
> important for better colorability criteria.  Function assign_hard_reg is 
> also based on this assumption.  Unfortunately, it is not true for 
> secondary allocation (after IRA IR flattening or during reload).  The 
> following patch solves this problem.
>
> The patch should be very safe but I am still testing it on x86/x86-64 
> bootstrap.
[...]
> Index: ira-color.c
> ===================================================================
> --- ira-color.c       (revision 171699)
> +++ ira-color.c       (working copy)
> @@ -1447,7 +1447,9 @@ update_conflict_hard_regno_costs (int *c
>  }
>  
>  /* Set up conflicting and profitable regs (through CONFLICT_REGS and
> -   PROFITABLE_REGS) for each object of allocno A.  */
> +   PROFITABLE_REGS) for each object of allocno A.  Remember that the
> +   profitable regs exclude hard regs which can not hold value of mode
> +   of allocno A.  */
>  static inline void
>  setup_conflict_profitable_regs (ira_allocno_t a, bool retry_p,
>                               HARD_REG_SET *conflict_regs,
> @@ -1463,8 +1465,13 @@ setup_conflict_profitable_regs (ira_allo
>        COPY_HARD_REG_SET (conflict_regs[i],
>                        OBJECT_TOTAL_CONFLICT_HARD_REGS (obj));
>        if (retry_p)
> -     COPY_HARD_REG_SET (profitable_regs[i],
> -                        reg_class_contents[ALLOCNO_CLASS (a)]);
> +     {
> +       COPY_HARD_REG_SET (profitable_regs[i],
> +                          reg_class_contents[ALLOCNO_CLASS (a)]);
> +       AND_COMPL_HARD_REG_SET (profitable_regs[i],
> +                               ira_prohibited_class_mode_regs
> +                               [ALLOCNO_CLASS (a)][ALLOCNO_MODE (a)]);
> +     }
>        else
>       COPY_HARD_REG_SET (profitable_regs[i],
>                          OBJECT_COLOR_DATA (obj)->profitable_hard_regs);

ira_prohibited_class_mode_regs is partly based on HARD_REGNO_MODE_OK,
which is really a property of the first register in a multi-register
group (rather than of every register in that group).  So is
ira_prohibited_class_mode_regs defined in the same way?
At the moment, check_hard_reg_p and setup_allocno_available_regs_num
test profitability for every register in the group:

        /* Checking only profitable hard regs.  */
        if (TEST_HARD_REG_BIT (conflict_regs[k], hard_regno + j)
            || ! TEST_HARD_REG_BIT (profitable_regs[k], hard_regno + j))
          break;
[...]
      for (j = 0; j < nregs; j++)
        {
        [...]
              /* Checking only profitable hard regs.  */
              if (TEST_HARD_REG_BIT (OBJECT_TOTAL_CONFLICT_HARD_REGS (obj),
                                     hard_regno + j)
                  || ! TEST_HARD_REG_BIT (obj_data->profitable_hard_regs,
                                          hard_regno + j))
                break;

So if you have a target in which double-word values have to start in
even registers, I think every odd-numbered bit of profitable_hard_regs
will be clear, and no register will seem profitable.  (I'm seeing this
on ARM with some VFP tests.)

Restricting the test to the first register fixes things for me,
but do we need to check something else for the j != 0 case?

Richard


gcc/
        * ira-color.c (check_hard_reg_p): Restrict the profitability check
        to the first register.
        (setup_allocno_available_regs_num): Likewise.

Index: gcc/ira-color.c
===================================================================
--- gcc/ira-color.c     (revision 171653)
+++ gcc/ira-color.c     (working copy)
@@ -1497,7 +1504,8 @@ check_hard_reg_p (ira_allocno_t a, int h
       for (k = set_to_test_start; k < set_to_test_end; k++)
        /* Checking only profitable hard regs.  */
        if (TEST_HARD_REG_BIT (conflict_regs[k], hard_regno + j)
-           || ! TEST_HARD_REG_BIT (profitable_regs[k], hard_regno + j))
+           || (j == 0
+               && ! TEST_HARD_REG_BIT (profitable_regs[k], hard_regno)))
          break;
       if (k != set_to_test_end)
        break;
@@ -2226,8 +2234,9 @@ setup_allocno_available_regs_num (ira_al
              /* Checking only profitable hard regs.  */
              if (TEST_HARD_REG_BIT (OBJECT_TOTAL_CONFLICT_HARD_REGS (obj),
                                     hard_regno + j)
-                 || ! TEST_HARD_REG_BIT (obj_data->profitable_hard_regs,
-                                         hard_regno + j))
+                 || (j == 0
+                     && ! TEST_HARD_REG_BIT (obj_data->profitable_hard_regs,
+                                             hard_regno)))
                break;
            }
          if (k != set_to_test_end)

Reply via email to