[IRA] : Fix PR 48927 - Issues with "enable" attribute and IRA register preferences

Submitted by Uros Bizjak on May 8, 2011, 7:39 p.m.

Details

Message ID BANLkTikW-_wFNNCo3BMe3VkwdJ-YOjwSVw@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak May 8, 2011, 7:39 p.m.
Hello!

Attached patch fixes changed register allocation where "enabled"
attribute is used. The core of the problem was with IRA, where IRA
does not look at "enabled" attribute when scanning through
alternatives string to perform various tasks (including register
allocation preferences).

Attached patch teaches IRA to handle "enabled" attribute. In effect,
the patch skips current alternative in the same way as when "#"
character is found.

2011-05-08  Uros Bizjak  <ubizjak@gmail.com>

	PR rtl-optimization/48927
	* ira-conflicts.c (commutative_constraint_p): Use
	recog_data.alternative_enabled_p to disable alternatives where
	"enabled" attribute is false.
	(get_dup_num): Ditto.
	* ira-lives.c (single_reg_class): Ditto.
	(ira_implicitly_set_insn_hard_regs): Ditto.

Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu
{,-m32} with and without -with-fpmath=avx. Also, the patch fixes both
i386 scan-asm failures when the patch to merge *vec_concatv4si_1_avx
and *vec_concatv4si_1 patterns is applied (please see the PR [1] for
details).

OK for mainline?

[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48927

Uros.

Comments

Bernd Schmidt May 9, 2011, 12:05 p.m.
On 05/08/2011 09:39 PM, Uros Bizjak wrote:

> Attached patch fixes changed register allocation where "enabled"
> attribute is used. The core of the problem was with IRA, where IRA
> does not look at "enabled" attribute when scanning through
> alternatives string to perform various tasks (including register
> allocation preferences).
> 
> Attached patch teaches IRA to handle "enabled" attribute. In effect,
> the patch skips current alternative in the same way as when "#"
> character is found.

Ok.


Bernd
Andreas Krebbel May 9, 2011, 12:46 p.m.
On Sun, May 08, 2011 at 09:39:09PM +0200, Uros Bizjak wrote:
> Hello!
> 
> Attached patch fixes changed register allocation where "enabled"
> attribute is used. The core of the problem was with IRA, where IRA
> does not look at "enabled" attribute when scanning through
> alternatives string to perform various tasks (including register
> allocation preferences).
> 
> Attached patch teaches IRA to handle "enabled" attribute. In effect,
> the patch skips current alternative in the same way as when "#"
> character is found.
> 
> 2011-05-08  Uros Bizjak  <ubizjak@gmail.com>
> 
> 	PR rtl-optimization/48927
> 	* ira-conflicts.c (commutative_constraint_p): Use
> 	recog_data.alternative_enabled_p to disable alternatives where
> 	"enabled" attribute is false.
> 	(get_dup_num): Ditto.
> 	* ira-lives.c (single_reg_class): Ditto.
> 	(ira_implicitly_set_insn_hard_regs): Ditto.
> 
> Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu
> {,-m32} with and without -with-fpmath=avx. Also, the patch fixes both
> i386 scan-asm failures when the patch to merge *vec_concatv4si_1_avx
> and *vec_concatv4si_1 patterns is applied (please see the PR [1] for
> details).
> 
> OK for mainline?

I've bootstrapped and regtested the patch on s390x. No regressions!

Bye,

-Andreas-
Vladimir Makarov May 10, 2011, 5:01 p.m.
On 05/08/2011 03:39 PM, Uros Bizjak wrote:
> Hello!
>
> Attached patch fixes changed register allocation where "enabled"
> attribute is used. The core of the problem was with IRA, where IRA
> does not look at "enabled" attribute when scanning through
> alternatives string to perform various tasks (including register
> allocation preferences).
>
> Attached patch teaches IRA to handle "enabled" attribute. In effect,
> the patch skips current alternative in the same way as when "#"
> character is found.
>
> 2011-05-08  Uros Bizjak<ubizjak@gmail.com>
>
> 	PR rtl-optimization/48927
> 	* ira-conflicts.c (commutative_constraint_p): Use
> 	recog_data.alternative_enabled_p to disable alternatives where
> 	"enabled" attribute is false.
> 	(get_dup_num): Ditto.
> 	* ira-lives.c (single_reg_class): Ditto.
> 	(ira_implicitly_set_insn_hard_regs): Ditto.
>
> Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu
> {,-m32} with and without -with-fpmath=avx. Also, the patch fixes both
> i386 scan-asm failures when the patch to merge *vec_concatv4si_1_avx
> and *vec_concatv4si_1 patterns is applied (please see the PR [1] for
> details).
>
> OK for mainline?
>
> [1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48927
>
It is ok for me too, Uros.  Sorry for the delay with the answer -- I was 
on vacation last week.

Attribute enabled is more and more actively used and I should check 
other places in IRA where the attribute should be checked.  Fortunately, 
ira-costs.c already uses it (it is most critical place).  Otherwise, we 
could have a very bad performance code.

Thanks for working on this.

Patch hide | download patch | download mbox

Index: ira-conflicts.c
===================================================================
--- ira-conflicts.c	(revision 173548)
+++ ira-conflicts.c	(working copy)
@@ -213,19 +213,22 @@  allocnos_conflict_for_copy_p (ira_allocn
 static bool
 commutative_constraint_p (const char *str)
 {
+  int curr_alt, c;
   bool ignore_p;
-  int c;
 
-  for (ignore_p = false;;)
+  for (ignore_p = false, curr_alt = 0;;)
     {
       c = *str;
       if (c == '\0')
 	break;
       str += CONSTRAINT_LEN (c, str);
-      if (c == '#')
+      if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
 	ignore_p = true;
       else if (c == ',')
-	ignore_p = false;
+	{
+	  curr_alt++;
+	  ignore_p = false;
+	}
       else if (! ignore_p)
 	{
 	  /* Usually `%' is the first constraint character but the
@@ -270,7 +273,7 @@  get_dup_num (int op_num, bool use_commut
       c = *str;
       if (c == '\0')
 	break;
-      if (c == '#')
+      if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
 	ignore_p = true;
       else if (c == ',')
 	{
Index: ira-lives.c
===================================================================
--- ira-lives.c	(revision 173548)
+++ ira-lives.c	(working copy)
@@ -726,18 +726,21 @@  mark_hard_reg_early_clobbers (rtx insn, 
 static enum reg_class
 single_reg_class (const char *constraints, rtx op, rtx equiv_const)
 {
-  int ignore_p;
+  int curr_alt, c;
+  bool ignore_p;
   enum reg_class cl, next_cl;
-  int c;
 
   cl = NO_REGS;
-  for (ignore_p = false;
+  for (ignore_p = false, curr_alt = 0;
        (c = *constraints);
        constraints += CONSTRAINT_LEN (c, constraints))
-    if (c == '#')
+    if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
       ignore_p = true;
     else if (c == ',')
-      ignore_p = false;
+      {
+	curr_alt++;
+	ignore_p = false;
+      }
     else if (! ignore_p)
       switch (c)
 	{
@@ -873,7 +876,7 @@  single_reg_operand_class (int op_num)
 void
 ira_implicitly_set_insn_hard_regs (HARD_REG_SET *set)
 {
-  int i, c, regno = 0;
+  int i, curr_alt, c, regno = 0;
   bool ignore_p;
   enum reg_class cl;
   rtx op;
@@ -895,11 +898,16 @@  ira_implicitly_set_insn_hard_regs (HARD_
 	  mode = (GET_CODE (op) == SCRATCH
 		  ? GET_MODE (op) : PSEUDO_REGNO_MODE (regno));
 	  cl = NO_REGS;
-	  for (ignore_p = false; (c = *p); p += CONSTRAINT_LEN (c, p))
-	    if (c == '#')
+	  for (ignore_p = false, curr_alt = 0;
+	       (c = *p);
+	       p += CONSTRAINT_LEN (c, p))
+	    if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
 	      ignore_p = true;
 	    else if (c == ',')
-	      ignore_p = false;
+	      {
+		curr_alt++;
+		ignore_p = false;
+	      }
 	    else if (! ignore_p)
 	      switch (c)
 		{