Re: RFA: patch to solve IRA PR48336, PR48342, PR48345
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 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_HAR
Re: RFA: patch to solve IRA PR48336, PR48342, PR48345
On Tue, 29 Mar 2011, Hans-Peter Nilsson wrote: > FWIW, I have a five regressions for cris-elf too appearing at > that RA change, but as they're wrong-code and noticed only at > execution, I'm going to wait analyzing further until this is > committed and caught by my autotester. Just a heads-up. Lest you lose sleep over this, it was fixed at r171716. ;) brgds, H-P
Re: RFA: patch to solve IRA PR48336, PR48342, PR48345
Hi Vlad,
First, I want to echo H-P's thanks for tackling this area. I just wondered:
Vladimir Makarov 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)
Re: RFA: patch to solve IRA PR48336, PR48342, PR48345
On Tue, 29 Mar 2011, Vladimir Makarov wrote: > 2011-03-29 Vladimir Makarov > > PR target/48336 > PR middle-end/4834 (A typo here, 48342. Or maybe also needed for 48334?) > PR rtl-optimization/48345 > * ira-color.c (setup_conflict_profitable_regs): Exclude prohibited > hard regs for given mode from profitable regs when doing secondary > allocation. FWIW, I have a five regressions for cris-elf too appearing at that RA change, but as they're wrong-code and noticed only at execution, I'm going to wait analyzing further until this is committed and caught by my autotester. Just a heads-up. brgds, H-P PS. thanks for sticking to such a high-bug-profile task. ;)
Re: RFA: patch to solve IRA PR48336, PR48342, PR48345
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 03/29/11 19:28, Vladimir Makarov wrote: > 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. > > Is it ok to commit the patch after successful bootsrapping? > > 2011-03-29 Vladimir Makarov > > PR target/48336 > PR middle-end/4834 > PR rtl-optimization/48345 > * ira-color.c (setup_conflict_profitable_regs): Exclude prohibited > hard regs for given mode from profitable regs when doing secondary > allocation. This is OK. Note the PR# in the ChangeLog (4834) should be (48342). jeff -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJNkomDAAoJEBRtltQi2kC7njQH/1GxaAuxFEZkYx8YdcYj//+c XyfW2qV0wC91w2GIlpX45zzZnINo5dySAIRerWxkv1dI4ycaxTcYyoWyZRWWUWHQ mKMYUZM8hmEdnNG/fur481cYo3lP45NmGzGFS5/lxyBJZXBaPk2gUJvYzLPFz/as 4ZJg3c5d05rw/1MdPOFwnKdzxk1TPciF7RP8uhFrEe1Uu8QyRf8ebtnpNyk93uF7 Z4/GafUBaSvYt/usRH4mijICE2cdMNrLq2S4A/RtQBhlOPvwSppIl3rT0kQnjAmw EUAXXGuKuHLEblzk1dVWpWvwMBXnw93qxSD+vkEbDFtcwzJqh5rqFJbVw4S/TQk= =Mkxi -END PGP SIGNATURE-
RFA: patch to solve IRA PR48336, PR48342, PR48345
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.
Is it ok to commit the patch after successful bootsrapping?
2011-03-29 Vladimir Makarov
PR target/48336
PR middle-end/4834
PR rtl-optimization/48345
* ira-color.c (setup_conflict_profitable_regs): Exclude prohibited
hard regs for given mode from profitable regs when doing secondary
allocation.
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);
