RE: [PATCH] Add force option to find_best_rename_reg in regrename pass

2014-12-05 Thread Thomas Preud'homme
> From: Eric Botcazou [mailto:ebotca...@adacore.com]
> Sent: Friday, December 05, 2014 4:40 PM
> 
> OK for mainline, but investigate whether you can better format the
> config/c6x/c6x.c line, for example:
> 
> + best_reg
> +   = find_rename_reg (this_head, super_class, &unavailable, old_reg,
> true);

Commited as suggested.

Thanks. Best regards,

Thomas 






Re: [PATCH] Add force option to find_best_rename_reg in regrename pass

2014-12-05 Thread Eric Botcazou
> 2014-11-26 Thomas Preud'homme thomas.preudho...@arm.com\
> 
>   * regrename.c (find_best_rename_reg): Rename to ...
>   (find_rename_reg): This. Also add a parameter to skip tick check.
>   * regrename.h: Likewise.
>   * config/c6x/c6x.c (try_rename_operands): Adapt to above renaming.

OK for mainline, but investigate whether you can better format the 
config/c6x/c6x.c line, for example:

+ best_reg
+   = find_rename_reg (this_head, super_class, &unavailable, old_reg, true);

-- 
Eric Botcazou


RE: [PATCH] Add force option to find_best_rename_reg in regrename pass

2014-12-04 Thread Thomas Preud'homme
Hi Eric,

Sorry for the delay, for some reasons despite being a recipient the mail
didn't hit my inbox but only my gcc-patches box.

> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Eric Botcazou
> 
> > 2014-11-14  Thomas Preud'homme  
> >
> > * regrename.c (find_best_rename_reg): Rename to ...
> > (find_rename_reg): This. Also add a parameter to skip tick check.
> > * regrename.h: Likewise.
> > * config/c6x/c6x.c: Adapt to above renaming.
> 
> Missing function in config/c6x/c6x.c entry.

Here is the proposed ChangeLog entry:

2014-11-26 Thomas Preud'homme thomas.preudho...@arm.com\

  * regrename.c (find_best_rename_reg): Rename to ...
  (find_rename_reg): This. Also add a parameter to skip tick check.
  * regrename.h: Likewise.
  * config/c6x/c6x.c (try_rename_operands): Adapt to above renaming.

> 
> Please write it like so:
> 
> if (!check_new_reg_p (old_reg, new_reg, this_head,
> *unavailable))
>   continue;
> 
> if (!best_rename)
>   return new_reg;
> 
> /* In the first pass, we force the renaming of registers that
>don't belong to PREFERRED_CLASS to registers that do, even
>though the latters were used not very long ago.  *
> if ((pass == 0
>&& !TEST_HARD_REG_BIT
> (reg_class_contents[preferred_class],
> best_new_reg))
> || tick[best_new_reg] > tick[new_reg]))
>   best_new_reg = new_reg;

Oh yes, much better indeed. I should have thought about this. Please find below
the patch reworked as above.

diff --git a/gcc/config/c6x/c6x.c b/gcc/config/c6x/c6x.c
index 06319d0..6aca1e3 100644
--- a/gcc/config/c6x/c6x.c
+++ b/gcc/config/c6x/c6x.c
@@ -3513,7 +3513,8 @@ try_rename_operands (rtx_insn *head, rtx_insn *tail, 
unit_req_table reqs,
   COMPL_HARD_REG_SET (unavailable, reg_class_contents[(int) super_class]);
 
   old_reg = this_head->regno;
-  best_reg = find_best_rename_reg (this_head, super_class, &unavailable, 
old_reg);
+  best_reg = find_rename_reg (this_head, super_class, &unavailable, old_reg,
+ true);
 
   regrename_do_replace (this_head, best_reg);
 
diff --git a/gcc/regrename.h b/gcc/regrename.h
index 03b7164..05c78ad 100644
--- a/gcc/regrename.h
+++ b/gcc/regrename.h
@@ -89,8 +89,8 @@ extern void regrename_init (bool);
 extern void regrename_finish (void);
 extern void regrename_analyze (bitmap);
 extern du_head_p regrename_chain_from_id (unsigned int);
-extern int find_best_rename_reg (du_head_p, enum reg_class, HARD_REG_SET *,
-int);
+extern int find_rename_reg (du_head_p, enum reg_class, HARD_REG_SET *, int,
+   bool);
 extern void regrename_do_replace (du_head_p, int);
 
 #endif
diff --git a/gcc/regrename.c b/gcc/regrename.c
index 66f562b..88321d0 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -357,11 +357,13 @@ check_new_reg_p (int reg ATTRIBUTE_UNUSED, int new_reg,
 /* For the chain THIS_HEAD, compute and return the best register to
rename to.  SUPER_CLASS is the superunion of register classes in
the chain.  UNAVAILABLE is a set of registers that cannot be used.
-   OLD_REG is the register currently used for the chain.  */
+   OLD_REG is the register currently used for the chain.  BEST_RENAME
+   controls whether the register chosen must be better than the
+   current one or just respect the given constraint.  */
 
 int
-find_best_rename_reg (du_head_p this_head, enum reg_class super_class,
- HARD_REG_SET *unavailable, int old_reg)
+find_rename_reg (du_head_p this_head, enum reg_class super_class,
+HARD_REG_SET *unavailable, int old_reg, bool best_rename)
 {
   bool has_preferred_class;
   enum reg_class preferred_class;
@@ -400,15 +402,19 @@ find_best_rename_reg (du_head_p this_head, enum reg_class 
super_class,
new_reg))
continue;
 
+ if (!check_new_reg_p (old_reg, new_reg, this_head, *unavailable))
+   continue;
+
+ if (!best_rename)
+   return new_reg;
+
  /* In the first pass, we force the renaming of registers that
 don't belong to PREFERRED_CLASS to registers that do, even
 though the latters were used not very long ago.  */
- if (check_new_reg_p (old_reg, new_reg, this_head,
-  *unavailable)
- && ((pass == 0
-  && !TEST_HARD_REG_BIT (reg_class_contents[preferred_class],
- best_new_reg))
- || tick[best_new_reg] > tick[new_reg]))
+ if ((pass == 0
+ && !TEST_HARD_REG_BIT (reg_class_contents[preferred_class],
+best_new_reg))
+ || tick[best_new_reg] > tick[new_reg])
best_new_reg = new_reg;
}
   if (pass == 0 &

Re: [PATCH] Add force option to find_best_rename_reg in regrename pass

2014-11-15 Thread Eric Botcazou
> It looks at register that respect the constraints of all the instructions in
> the set and tries to pick one in the preferred class for all the
> instructions involved. This is generally useful for any pass that wants to
> do register renaming. However it also contains some logic to only select
> the register that also haven't been used for a longer time than the
> register that should be replaced. This bit is specific to the register
> renaming pass and makes the function unusable for this new pass as a result
> which forces us to do a copy of the function.
>
> This patch adds an extra parameter to skip this check and only consider the
> constraints and tries to pick a register in the preferred class.

OK on principle but...

> 2014-11-14  Thomas Preud'homme  
> 
> * regrename.c (find_best_rename_reg): Rename to ...
> (find_rename_reg): This. Also add a parameter to skip tick check.
> * regrename.h: Likewise.
> * config/c6x/c6x.c: Adapt to above renaming.

Missing function in config/c6x/c6x.c entry.

> @@ -408,8 +410,13 @@ find_best_rename_reg (du_head_p this_head, enum
> reg_class super_class, && ((pass == 0
>  && !TEST_HARD_REG_BIT (reg_class_contents[preferred_class],
> best_new_reg))
> -   || tick[best_new_reg] > tick[new_reg]))
> - best_new_reg = new_reg;
> +   || !best_rename || tick[best_new_reg] > tick[new_reg]))
> + {
> +   if (best_rename)
> + best_new_reg = new_reg;
> +   else
> + return new_reg;
> + }
>   }
>if (pass == 0 && best_new_reg != old_reg)
>   break;

Please write it like so:

  if (!check_new_reg_p (old_reg, new_reg, this_head, *unavailable))
continue;

  if (!best_rename)
return new_reg;

  /* In the first pass, we force the renaming of registers that
 don't belong to PREFERRED_CLASS to registers that do, even
 though the latters were used not very long ago.  *
  if ((pass == 0
 && !TEST_HARD_REG_BIT (reg_class_contents[preferred_class],
  best_new_reg))
  || tick[best_new_reg] > tick[new_reg]))
best_new_reg = new_reg;

-- 
Eric Botcazou