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

login
register
mail settings
Submitter Uros Bizjak
Date May 8, 2011, 7:39 p.m.
Message ID <BANLkTikW-_wFNNCo3BMe3VkwdJ-YOjwSVw@mail.gmail.com>
Download mbox | patch
Permalink /patch/94602/
State New
Headers show

Comments

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.
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

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)
 		{