Patchwork rs6000: Tighten register predicates

login
register
mail settings
Submitter Segher Boessenkool
Date Sept. 22, 2012, 2:11 a.m.
Message ID <c9db2ccc25ce151e772e2f7ccb7bb6acdb0e2adb.1348279211.git.segher@kernel.crashing.org>
Download mbox | patch
Permalink /patch/186035/
State New
Headers show

Comments

Segher Boessenkool - Sept. 22, 2012, 2:11 a.m.
Currently, the register predicates allow a subreg of anything,
including memory or the wrong kind of register.  Most other ports
do not allow this.  As far as I can see, this doesn't help the
compiler generate better code, potentially the opposite.

This also causes a problem for my (upcoming) add-with-carry work.
The patterns for the add-with-carry instructions are each a parallel
of two sets, one of a GPR and one of CA.  Sometimes one of those
sets isn't used, for example for most ADDE instructions, and combine
will try e.g.  (set (reg:P xxx) (reg:P CA_REGNO))  which fails
because that isn't an existing instruction.  But when it tries
(set (reg:SI xxx) (subreg:SI (reg:DI CA_REGNO) 4))  it currently
succeeds, because it looks like movsi2; but of course it ICEs
later on.

This patch tightens the register predicates to only allow a subreg
of the kind of registers the predicates allows.  The generated code
for e.g. gpc_reg_operand looks a bit worse, but benchmarking shows
it to be a wash (average of three runs on combine.ii: original
0m13.014s, patched 0m13.003s).

Tested on powerpc64-linux --enable-languages=c,c++,fortran;
no regressions.  Okay to apply?


Segher


2012-09-21  Segher Boessenkool  <segher@kernel.crashing.org>

gcc/
	* config/rs6000/predicates.md (altivec_register_operand,
	vsx_register_operand, vfloat_operand, vint_operand,
	vlogical_operand, gpc_reg_operand, cc_reg_operand,
	cc_reg_not_cr0_operand, cc_reg_not_micro_cr0_operand):
	If op is a SUBREG, consider its SUBREG_REG instead.

---
 gcc/config/rs6000/predicates.md |  166 +++++++++++++++++++++++++++++----------
 1 files changed, 124 insertions(+), 42 deletions(-)
David Edelsohn - Sept. 22, 2012, 8:06 p.m.
On Fri, Sep 21, 2012 at 10:11 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> Currently, the register predicates allow a subreg of anything,
> including memory or the wrong kind of register.  Most other ports
> do not allow this.  As far as I can see, this doesn't help the
> compiler generate better code, potentially the opposite.
>
> This also causes a problem for my (upcoming) add-with-carry work.
> The patterns for the add-with-carry instructions are each a parallel
> of two sets, one of a GPR and one of CA.  Sometimes one of those
> sets isn't used, for example for most ADDE instructions, and combine
> will try e.g.  (set (reg:P xxx) (reg:P CA_REGNO))  which fails
> because that isn't an existing instruction.  But when it tries
> (set (reg:SI xxx) (subreg:SI (reg:DI CA_REGNO) 4))  it currently
> succeeds, because it looks like movsi2; but of course it ICEs
> later on.
>
> This patch tightens the register predicates to only allow a subreg
> of the kind of registers the predicates allows.  The generated code
> for e.g. gpc_reg_operand looks a bit worse, but benchmarking shows
> it to be a wash (average of three runs on combine.ii: original
> 0m13.014s, patched 0m13.003s).
>
> Tested on powerpc64-linux --enable-languages=c,c++,fortran;
> no regressions.  Okay to apply?
>
>
> Segher
>
>
> 2012-09-21  Segher Boessenkool  <segher@kernel.crashing.org>
>
> gcc/
>         * config/rs6000/predicates.md (altivec_register_operand,
>         vsx_register_operand, vfloat_operand, vint_operand,
>         vlogical_operand, gpc_reg_operand, cc_reg_operand,
>         cc_reg_not_cr0_operand, cc_reg_not_micro_cr0_operand):
>         If op is a SUBREG, consider its SUBREG_REG instead.

Okay.

Thanks, David

Patch

diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index e2c3e70..12b7527 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -30,44 +30,89 @@  (define_predicate "count_register_operand"
   (and (match_code "reg")
        (match_test "REGNO (op) == CTR_REGNO
 		    || REGNO (op) > LAST_VIRTUAL_REGISTER")))
-  
+
 ;; Return 1 if op is an Altivec register.
 (define_predicate "altivec_register_operand"
-   (and (match_operand 0 "register_operand")
-	(match_test "GET_CODE (op) != REG
-		     || ALTIVEC_REGNO_P (REGNO (op))
-		     || REGNO (op) > LAST_VIRTUAL_REGISTER")))
+  (match_operand 0 "register_operand")
+{
+  if (GET_CODE (op) == SUBREG)
+    op = SUBREG_REG (op);
+
+  if (!REG_P (op))
+    return 0;
+
+  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+    return 1;
+
+  return ALTIVEC_REGNO_P (REGNO (op));
+})
 
 ;; Return 1 if op is a VSX register.
 (define_predicate "vsx_register_operand"
-   (and (match_operand 0 "register_operand")
-	(match_test "GET_CODE (op) != REG
-		     || VSX_REGNO_P (REGNO (op))
-		     || REGNO (op) > LAST_VIRTUAL_REGISTER")))
+  (match_operand 0 "register_operand")
+{
+  if (GET_CODE (op) == SUBREG)
+    op = SUBREG_REG (op);
+
+  if (!REG_P (op))
+    return 0;
+
+  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+    return 1;
+
+  return VSX_REGNO_P (REGNO (op));
+})
 
 ;; Return 1 if op is a vector register that operates on floating point vectors
 ;; (either altivec or VSX).
 (define_predicate "vfloat_operand"
-   (and (match_operand 0 "register_operand")
-	(match_test "GET_CODE (op) != REG
-		     || VFLOAT_REGNO_P (REGNO (op))
-		     || REGNO (op) > LAST_VIRTUAL_REGISTER")))
+  (match_operand 0 "register_operand")
+{
+  if (GET_CODE (op) == SUBREG)
+    op = SUBREG_REG (op);
+
+  if (!REG_P (op))
+    return 0;
+
+  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+    return 1;
+
+  return VFLOAT_REGNO_P (REGNO (op));
+})
 
 ;; Return 1 if op is a vector register that operates on integer vectors
 ;; (only altivec, VSX doesn't support integer vectors)
 (define_predicate "vint_operand"
-   (and (match_operand 0 "register_operand")
-	(match_test "GET_CODE (op) != REG
-		     || VINT_REGNO_P (REGNO (op))
-		     || REGNO (op) > LAST_VIRTUAL_REGISTER")))
+  (match_operand 0 "register_operand")
+{
+  if (GET_CODE (op) == SUBREG)
+    op = SUBREG_REG (op);
+
+  if (!REG_P (op))
+    return 0;
+
+  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+    return 1;
+
+  return VINT_REGNO_P (REGNO (op));
+})
 
 ;; Return 1 if op is a vector register to do logical operations on (and, or,
 ;; xor, etc.)
 (define_predicate "vlogical_operand"
-   (and (match_operand 0 "register_operand")
-	(match_test "GET_CODE (op) != REG
-		     || VLOGICAL_REGNO_P (REGNO (op))
-		     || REGNO (op) > LAST_VIRTUAL_REGISTER")))
+  (match_operand 0 "register_operand")
+{
+  if (GET_CODE (op) == SUBREG)
+    op = SUBREG_REG (op);
+
+  if (!REG_P (op))
+    return 0;
+
+  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+    return 1;
+
+  return VLOGICAL_REGNO_P (REGNO (op));
+})
 
 ;; Return 1 if op is the carry register.
 (define_predicate "ca_operand"
@@ -123,36 +168,73 @@  (define_predicate "const_2_to_3_operand"
 
 ;; Return 1 if op is a register that is not special.
 (define_predicate "gpc_reg_operand"
-   (and (match_operand 0 "register_operand")
-	(match_test "(GET_CODE (op) != REG
-		      || (REGNO (op) >= ARG_POINTER_REGNUM
-			  && !CA_REGNO_P (REGNO (op)))
-		      || INT_REGNO_P (REGNO (op))
-		      || FP_REGNO_P (REGNO (op)))
-		     && !((TARGET_E500_DOUBLE || TARGET_SPE)
-			  && invalid_e500_subreg (op, mode))")))
+  (match_operand 0 "register_operand")
+{
+  if ((TARGET_E500_DOUBLE || TARGET_SPE) && invalid_e500_subreg (op, mode))
+    return 0;
+
+  if (GET_CODE (op) == SUBREG)
+    op = SUBREG_REG (op);
+
+  if (!REG_P (op))
+    return 0;
+
+  if (REGNO (op) >= ARG_POINTER_REGNUM && !CA_REGNO_P (REGNO (op)))
+    return 1;
+
+  return INT_REGNO_P (REGNO (op)) || FP_REGNO_P (REGNO (op));
+})
 
 ;; Return 1 if op is a register that is a condition register field.
 (define_predicate "cc_reg_operand"
-   (and (match_operand 0 "register_operand")
-	(match_test "GET_CODE (op) != REG
-		     || REGNO (op) > LAST_VIRTUAL_REGISTER
-		     || CR_REGNO_P (REGNO (op))")))
+  (match_operand 0 "register_operand")
+{
+  if (GET_CODE (op) == SUBREG)
+    op = SUBREG_REG (op);
+
+  if (!REG_P (op))
+    return 0;
+
+  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+    return 1;
+
+  return CR_REGNO_P (REGNO (op));
+})
 
 ;; Return 1 if op is a register that is a condition register field not cr0.
 (define_predicate "cc_reg_not_cr0_operand"
-   (and (match_operand 0 "register_operand")
-	(match_test "GET_CODE (op) != REG
-		     || REGNO (op) > LAST_VIRTUAL_REGISTER
-		     || CR_REGNO_NOT_CR0_P (REGNO (op))")))
+  (match_operand 0 "register_operand")
+{
+  if (GET_CODE (op) == SUBREG)
+    op = SUBREG_REG (op);
+
+  if (!REG_P (op))
+    return 0;
+
+  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+    return 1;
+
+  return CR_REGNO_NOT_CR0_P (REGNO (op));
+})
 
 ;; Return 1 if op is a register that is a condition register field and if generating microcode, not cr0.
 (define_predicate "cc_reg_not_micro_cr0_operand"
-   (and (match_operand 0 "register_operand")
-	(match_test "GET_CODE (op) != REG
-		     || REGNO (op) > LAST_VIRTUAL_REGISTER
-		     || (rs6000_gen_cell_microcode && CR_REGNO_NOT_CR0_P (REGNO (op)))
-		     || (!rs6000_gen_cell_microcode && CR_REGNO_P (REGNO (op)))")))
+  (match_operand 0 "register_operand")
+{
+  if (GET_CODE (op) == SUBREG)
+    op = SUBREG_REG (op);
+
+  if (!REG_P (op))
+    return 0;
+
+  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+    return 1;
+
+  if (rs6000_gen_cell_microcode)
+    return CR_REGNO_NOT_CR0_P (REGNO (op));
+  else
+    return CR_REGNO_P (REGNO (op));
+})
 
 ;; Return 1 if op is a constant integer valid for D field
 ;; or non-special register register.