Re: RFC: PATCH: Remove 26 element limit in vector

[H.J. Lu]

On Thu, Mar 31, 2011 at 5:05 AM, Kenneth Zadeck
zad...@naturalbridge.com wrote:
 we hit this limit trying to write the explicit semantics for a
 vec_interleave_evenv32qi.

 ;;(define_insn vec_interleave_evenv32qi
 ;;  [(set (match_operand:V32QI 0 register_operand =r)
 ;;    (vec_select:V32QI
 ;;      (vec_concat:V64QI
 ;;        (match_operand:V32QI 1 register_operand 0)
 ;;        (match_operand:V32QI 2 register_operand r))
 ;;      (parallel [(const_int  0) (const_int 32)
 ;;             (const_int  2) (const_int 34)
 ;;             (const_int  4) (const_int 36)
 ;;             (const_int  6) (const_int 38)
 ;;             (const_int  8) (const_int 40)
 ;;             (const_int 10) (const_int 42)
 ;;             (const_int 12) (const_int 44)
 ;;             (const_int 14) (const_int 46)
 ;;             (const_int 16) (const_int 48)
 ;;             (const_int 18) (const_int 50)
 ;;             (const_int 20) (const_int 52)
 ;;             (const_int 22) (const_int 54)
 ;;             (const_int 24) (const_int 56)
 ;;             (const_int 26) (const_int 58)
 ;;             (const_int 28) (const_int 60)
 ;;             (const_int 30) (const_int 62)])))]
 ;;  
 ;;  rimihv\t%0,%2,8,15,8
 ;;  [(set_attr type rimi)])


 kenny

 On 03/31/2011 06:16 AM, Mike Stump wrote:

 On Mar 31, 2011, at 1:41 AM, Richard Guenther wrote:

 On Wed, Mar 30, 2011 at 8:09 PM, H.J. Luhongjiu...@intel.com  wrote:

 On Wed, Mar 30, 2011 at 08:02:38AM -0700, H.J. Lu wrote:

 Hi,

 Currently, we limit XVECEXP to 26 elements in machine description
 since we use letters 'a' to 'z' to encode them.  I don't see any
 reason why we can't go beyond 'z'.  This patch removes this
 restriction.
 Any comments?

 That was wrong.  The problem is in vector elements.  This patch passes
 bootstrap.  Any comments?

 Do you really need it?

 I'm trying to recall if this is the limit Kenny and I hit  If so,
 annoying.  Kenny could confirm if it was.  gcc's general strategy of, no
 fixed N gives gcc a certain flexibility that is very nice to have, on those
 general grounds, I kinda liked this patch.


Is my patch OK to install?

Thanks.

-- 
H.J.

Re: RFC: PATCH: Remove 26 element limit in vector

[Richard Guenther]

On Wed, Mar 30, 2011 at 8:09 PM, H.J. Lu hongjiu...@intel.com wrote:
 On Wed, Mar 30, 2011 at 08:02:38AM -0700, H.J. Lu wrote:
 Hi,

 Currently, we limit XVECEXP to 26 elements in machine description
 since we use letters 'a' to 'z' to encode them.  I don't see any
 reason why we can't go beyond 'z'.  This patch removes this restriction.
 Any comments?


 That was wrong.  The problem is in vector elements.  This patch passes
 bootstrap.  Any comments?

Do you really need it?

Richard.

 Thanks.


 H.J.
 ---
 2011-03-30  H.J. Lu  hongjiu...@intel.com

        * genrecog.c (VECTOR_ELEMENT_BASE): New.
        (add_to_sequence): Add assert.  Use VECTOR_ELEMENT_BASE to
        encode vector elements.
        (change_state): Check and support VECTOR_ELEMENT_BASE.
        (make_insn_sequence): Add assert.

 diff --git a/gcc/genrecog.c b/gcc/genrecog.c
 index 74dd0a7..40e9c4d 100644
 --- a/gcc/genrecog.c
 +++ b/gcc/genrecog.c
 @@ -465,6 +465,9 @@ extern void debug_decision
   (struct decision *);
  extern void debug_decision_list
   (struct decision *);
 +
 +/* The base of vector element.  */
 +#define VECTOR_ELEMENT_BASE 0x80

  /* Create a new node in sequence after LAST.  */

 @@ -912,6 +915,7 @@ add_to_sequence (rtx pattern, struct decision_head *last, 
 const char *position,
              /* Which insn we're looking at is represented by A-Z. We don't
                 ever use 'A', however; it is always implied.  */

 +             gcc_assert (i  26);
              subpos[depth] = (i  0 ? 'A' + i : 0);
              sub = add_to_sequence (XVECEXP (pattern, 0, i),
                                     last, subpos, insn_type, 0);
 @@ -1002,6 +1006,9 @@ add_to_sequence (rtx pattern, struct decision_head 
 *last, const char *position,
            char base = (was_code == MATCH_OPERATOR ? '0' : 'a');
            for (i = 0; i  (size_t) XVECLEN (pattern, 2); i++)
              {
 +               gcc_assert (was_code == MATCH_OPERATOR
 +                           ? ISDIGIT (i + base)
 +                           : ISLOWER (i + base));
                subpos[depth] = i + base;
                sub = add_to_sequence (XVECEXP (pattern, 2, i),
                                       sub-success, subpos, insn_type, 0);
 @@ -1102,7 +1109,9 @@ add_to_sequence (rtx pattern, struct decision_head 
 *last, const char *position,
            int j;
            for (j = 0; j  XVECLEN (pattern, i); j++)
              {
 -               subpos[depth] = 'a' + j;
 +               int val = j + VECTOR_ELEMENT_BASE;
 +               gcc_assert (val = UCHAR_MAX);
 +               subpos[depth] = val;
                sub = add_to_sequence (XVECEXP (pattern, i, j),
                                       sub-success, subpos, insn_type, 0);
              }
 @@ -1779,6 +1788,10 @@ change_state (const char *oldpos, const char *newpos, 
 const char *indent)
       else if (ISLOWER (newpos[depth]))
        printf (%sx%d = XVECEXP (x%d, 0, %d);\n,
                indent, depth + 1, depth, newpos[depth] - 'a');
 +      else if (((unsigned char) newpos[depth]) = VECTOR_ELEMENT_BASE)
 +       printf (%sx%d = XVECEXP (x%d, 0, %d);\n,
 +               indent, depth + 1, depth,
 +               ((unsigned char) newpos[depth]) - VECTOR_ELEMENT_BASE);
       else
        printf (%sx%d = XEXP (x%d, %c);\n,
                indent, depth + 1, depth, newpos[depth]);
 @@ -2528,6 +2541,7 @@ make_insn_sequence (rtx insn, enum routine_type type)
        }
       XVECLEN (x, 0) = j;

 +      gcc_assert ((j - 1)  26);
       c_test_pos[0] = 'A' + j - 1;
       c_test_pos[1] = '\0';
     }

Re: RFC: PATCH: Remove 26 element limit in vector

[Richard Sandiford]

Richard Guenther richard.guent...@gmail.com writes:
 On Wed, Mar 30, 2011 at 8:09 PM, H.J. Lu hongjiu...@intel.com wrote:
 On Wed, Mar 30, 2011 at 08:02:38AM -0700, H.J. Lu wrote:
 Hi,

 Currently, we limit XVECEXP to 26 elements in machine description
 since we use letters 'a' to 'z' to encode them.  I don't see any
 reason why we can't go beyond 'z'.  This patch removes this restriction.
 Any comments?


 That was wrong.  The problem is in vector elements.  This patch passes
 bootstrap.  Any comments?

 Do you really need it?

ISTR at least two separate projects have hit the limit.  This solution
seems very hackish though (as well as being ASCII-specific).  A better
fix might be to replace char with a proper type, such as a union and
discriminator.

Richard

Re: RFC: PATCH: Remove 26 element limit in vector

[Kenneth Zadeck]

we hit this limit trying to write the explicit semantics for a 
vec_interleave_evenv32qi.


;;(define_insn vec_interleave_evenv32qi
;;  [(set (match_operand:V32QI 0 register_operand =r)
;;(vec_select:V32QI
;;  (vec_concat:V64QI
;;(match_operand:V32QI 1 register_operand 0)
;;(match_operand:V32QI 2 register_operand r))
;;  (parallel [(const_int  0) (const_int 32)
;; (const_int  2) (const_int 34)
;; (const_int  4) (const_int 36)
;; (const_int  6) (const_int 38)
;; (const_int  8) (const_int 40)
;; (const_int 10) (const_int 42)
;; (const_int 12) (const_int 44)
;; (const_int 14) (const_int 46)
;; (const_int 16) (const_int 48)
;; (const_int 18) (const_int 50)
;; (const_int 20) (const_int 52)
;; (const_int 22) (const_int 54)
;; (const_int 24) (const_int 56)
;; (const_int 26) (const_int 58)
;; (const_int 28) (const_int 60)
;; (const_int 30) (const_int 62)])))]
;;  
;;  rimihv\t%0,%2,8,15,8
;;  [(set_attr type rimi)])


kenny

On 03/31/2011 06:16 AM, Mike Stump wrote:

On Mar 31, 2011, at 1:41 AM, Richard Guenther wrote:

On Wed, Mar 30, 2011 at 8:09 PM, H.J. Luhongjiu...@intel.com  wrote:

On Wed, Mar 30, 2011 at 08:02:38AM -0700, H.J. Lu wrote:

Hi,

Currently, we limit XVECEXP to 26 elements in machine description
since we use letters 'a' to 'z' to encode them.  I don't see any
reason why we can't go beyond 'z'.  This patch removes this restriction.
Any comments?


That was wrong.  The problem is in vector elements.  This patch passes
bootstrap.  Any comments?

Do you really need it?

I'm trying to recall if this is the limit Kenny and I hit  If so, annoying. 
 Kenny could confirm if it was.  gcc's general strategy of, no fixed N gives 
gcc a certain flexibility that is very nice to have, on those general grounds, 
I kinda liked this patch.

RFC: PATCH: Remove 26 element limit in vector

[H.J. Lu]

On Wed, Mar 30, 2011 at 08:02:38AM -0700, H.J. Lu wrote:
 Hi,
 
 Currently, we limit XVECEXP to 26 elements in machine description
 since we use letters 'a' to 'z' to encode them.  I don't see any
 reason why we can't go beyond 'z'.  This patch removes this restriction.
 Any comments?
 

That was wrong.  The problem is in vector elements.  This patch passes
bootstrap.  Any comments?

Thanks.


H.J.
---
2011-03-30  H.J. Lu  hongjiu...@intel.com

* genrecog.c (VECTOR_ELEMENT_BASE): New.
(add_to_sequence): Add assert.  Use VECTOR_ELEMENT_BASE to
encode vector elements.
(change_state): Check and support VECTOR_ELEMENT_BASE.
(make_insn_sequence): Add assert.

diff --git a/gcc/genrecog.c b/gcc/genrecog.c
index 74dd0a7..40e9c4d 100644
--- a/gcc/genrecog.c
+++ b/gcc/genrecog.c
@@ -465,6 +465,9 @@ extern void debug_decision
   (struct decision *);
 extern void debug_decision_list
   (struct decision *);
+
+/* The base of vector element.  */
+#define VECTOR_ELEMENT_BASE 0x80
 
 /* Create a new node in sequence after LAST.  */
 
@@ -912,6 +915,7 @@ add_to_sequence (rtx pattern, struct decision_head *last, 
const char *position,
  /* Which insn we're looking at is represented by A-Z. We don't
 ever use 'A', however; it is always implied.  */
 
+ gcc_assert (i  26);
  subpos[depth] = (i  0 ? 'A' + i : 0);
  sub = add_to_sequence (XVECEXP (pattern, 0, i),
 last, subpos, insn_type, 0);
@@ -1002,6 +1006,9 @@ add_to_sequence (rtx pattern, struct decision_head *last, 
const char *position,
char base = (was_code == MATCH_OPERATOR ? '0' : 'a');
for (i = 0; i  (size_t) XVECLEN (pattern, 2); i++)
  {
+   gcc_assert (was_code == MATCH_OPERATOR
+   ? ISDIGIT (i + base)
+   : ISLOWER (i + base));
subpos[depth] = i + base;
sub = add_to_sequence (XVECEXP (pattern, 2, i),
   sub-success, subpos, insn_type, 0);
@@ -1102,7 +1109,9 @@ add_to_sequence (rtx pattern, struct decision_head *last, 
const char *position,
int j;
for (j = 0; j  XVECLEN (pattern, i); j++)
  {
-   subpos[depth] = 'a' + j;
+   int val = j + VECTOR_ELEMENT_BASE;
+   gcc_assert (val = UCHAR_MAX);
+   subpos[depth] = val;
sub = add_to_sequence (XVECEXP (pattern, i, j),
   sub-success, subpos, insn_type, 0);
  }
@@ -1779,6 +1788,10 @@ change_state (const char *oldpos, const char *newpos, 
const char *indent)
   else if (ISLOWER (newpos[depth]))
printf (%sx%d = XVECEXP (x%d, 0, %d);\n,
indent, depth + 1, depth, newpos[depth] - 'a');
+  else if (((unsigned char) newpos[depth]) = VECTOR_ELEMENT_BASE)
+   printf (%sx%d = XVECEXP (x%d, 0, %d);\n,
+   indent, depth + 1, depth,
+   ((unsigned char) newpos[depth]) - VECTOR_ELEMENT_BASE);
   else
printf (%sx%d = XEXP (x%d, %c);\n,
indent, depth + 1, depth, newpos[depth]);
@@ -2528,6 +2541,7 @@ make_insn_sequence (rtx insn, enum routine_type type)
}
   XVECLEN (x, 0) = j;
 
+  gcc_assert ((j - 1)  26);
   c_test_pos[0] = 'A' + j - 1;
   c_test_pos[1] = '\0';
 }