| Message ID | g4r59ooqqc.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 03/30/2011 06:19 AM, Richard Sandiford wrote: > Hi Vlad, > > First, I want to echo H-P's thanks for tackling this area. I just wondered: > > Vladimir Makarov<vmakarov@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, thanks very much for pointing this out. I am agree with you. I think your patch is ok. Although I need some time to check it. Right now I am overwhelmed by # of other bug reports. Another thing bothering me is a new colorability test for pseudos which should start on even or odd hard register. I have suspicion that it is not ok for now. I need some time to check and think about it. I hope that I finish my urgent work on bug reports this week and start to work on your patch and the another issue on next week. > 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)
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)