diff mbox

support operator list

Message ID CAJXstsBPLEa7zVV3baiwW-z3_472uE_zpnWWx2xtPW0ez3KC6A@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni Oct. 26, 2014, 6:40 a.m. UTC 
Fixed a silly mistake in match-bitwise.pd (I had put ')' at wrong place).
This patch also checks if operator-list is used outside for-pattern
(in parser::parse_operation).

* genmatch.c
    (user_id): Add new member is_oper_list.
    (user_id::user_id): Add new default argument.
    (parser::parse_operator_list): New member function in parser.
    (add_substitute): New function.
    (flatten_substitutes): Likewise.
    (parser::parse_for): Call add_substitute.
    (parser::parse_pattern): Call parser::parse_operator_list.
    (parser::parse_operation): Put check for operator-list.

* match-bitwise.pd
    (bitwise_ors): New operator-list.
    (bitwise_ops): Likewise.
    Use bitwise_ors and bitwise_ops in patterns.

* match-comparison.pd
     (eq_ops): New operator-list.
     (cmp_ops): Likewise.
     Use cmp_ops and eq_ops in patterns.

Thanks,
Prathamesh



On Sat, Oct 25, 2014 at 10:44 PM, Prathamesh Kulkarni
<bilbotheelffriend@gmail.com> wrote:
> Hi,
>     This patch adds support for operator-lists, and uses
> them in match-bitwise.pd and match-comparison.pd
>
> * genmatch.c
>     (parser::parse_operator_list): New member function in parser.
>     (add_substitute): New function.
>     (flatten_substitutes): Likewise.
>     (parser::parse_for): Call add_substitute.
>     (parser::parse_pattern): Call parser::parse_operator_list.
>
> * match-bitwise.pd
>     (bitwise_ors): New operator-list.
>     (bitwise_ops): Likewise.
>     Use bitwise_ors and bitwise_ops in patterns.
>
> * match-comparison.pd
>      (eq_ops): New operator-list.
>      (cmp_ops): Likewise.
>      Use cmp_ops and eq_ops in patterns.
>
> Thanks,
> Prathamesh

Comments

Richard Biener Oct. 28, 2014, 2:02 p.m. UTC | #1 
On Sun, Oct 26, 2014 at 7:40 AM, Prathamesh Kulkarni
<bilbotheelffriend@gmail.com> wrote:
> Fixed a silly mistake in match-bitwise.pd (I had put ')' at wrong place).
> This patch also checks if operator-list is used outside for-pattern
> (in parser::parse_operation).

With recent discussion on how to use operator-lists - thus

(for (oplist1 oplist2 ...)
  (simplify (oplist1 @0 @1)...

it shouldn't be necessary to have an is_oper_list member in user_id
as all user_ids are operator-lists this way (just temporarily defined
in for scope eventually).

-         if (arity == -1)
-           arity = idb->nargs;
-         else if (idb->nargs == -1)
-           ;
-         else if (idb->nargs != arity)
-           fatal_at (token, "operator '%s' with arity %d does not match "
-                     "others with arity %d", oper, idb->nargs, arity);
+         vec<id_base *> substitutes = vNULL;
+         if (idb->kind == id_base::USER)
+           substitutes = (as_a<user_id *> (idb))->substitutes;
+         else
+           substitutes.safe_push (idb);

-         op->substitutes.safe_push (idb);
+         for (unsigned si = 0; si < substitutes.length (); ++si)
+           if (!add_substitute (op, substitutes[si], arity))
+             fatal_at (token, "operator '%s' with arity %d does not match "
+                              "others with arity %d", oper, idb->nargs, arity);

an operator-list user_id should have a proper arity computed.  With
the syntax change this probably will look differently anyway.

+void
+parser::parse_operator_list ()
+{
+  const char *id = get_ident ();
+  user_id *op = new user_id (id, true);
+  vec<const cpp_token *> user_id_tokens = vNULL;
+
+  id_base **slot = operators->find_slot_with_hash (op, op->hashval, INSERT);
+  if (*slot)
+    fatal ("operator %s already defined", id);

use get_operator (id) != NULL, otherwise same issue as with the
other already defined check we just fixed.

+      id_base *p = get_operator (oper);
+      if (p == 0)
+       fatal_at (token, "%s is not defined", oper);

I'd say "no such operator '%s'"

+  flatten_substitutes (ids, op->substitutes);

huh, so you support

(define_operator_list list1 a b)
(define_operator_list list2 list1 c)

?  Well, ok.  Any reason to not splice in during parsing?  I also
miss a check for matching arity and computing arity of the
operator list.  That's all be best done here I think, in the loop
parsing the identifiers.

@@ -23,7 +26,7 @@
    binary operation result instead of to the operands.  This allows
    to combine successive conversions and bitwise binary operations.  */
 /* We combine the above two cases by using a conditional convert.  */
-(for bitop (bit_and bit_ior bit_xor)
+(for bitop (bitwise_ops)

as discussed this will change to

(for (bitwise_ops)

instead.  Btw - I'm not sure that using

(define_operator_list bitwise_ors bit_ior bit_xor)
(define_operator_list bitwise_ops bit_and bitwise_ors)

will improve readability.  I'd like to see it mostly for comparisons
and math builtin variants.  For example we have the well-known
term comparison_class, so

(define_operator_list comparison_class lt le eq ne ge gt unordered
ordered unlt unle ungt unge uneq ltgt)

or rather shorter, 'cc' instead of 'comparison_class'.

Thanks,
Richard.

> * genmatch.c
>     (user_id): Add new member is_oper_list.
>     (user_id::user_id): Add new default argument.
>     (parser::parse_operator_list): New member function in parser.
>     (add_substitute): New function.
>     (flatten_substitutes): Likewise.
>     (parser::parse_for): Call add_substitute.
>     (parser::parse_pattern): Call parser::parse_operator_list.
>     (parser::parse_operation): Put check for operator-list.
>
> * match-bitwise.pd
>     (bitwise_ors): New operator-list.
>     (bitwise_ops): Likewise.
>     Use bitwise_ors and bitwise_ops in patterns.
>
> * match-comparison.pd
>      (eq_ops): New operator-list.
>      (cmp_ops): Likewise.
>      Use cmp_ops and eq_ops in patterns.
>
> Thanks,
> Prathamesh
>
>
>
> On Sat, Oct 25, 2014 at 10:44 PM, Prathamesh Kulkarni
> <bilbotheelffriend@gmail.com> wrote:
>> Hi,
>>     This patch adds support for operator-lists, and uses
>> them in match-bitwise.pd and match-comparison.pd
>>
>> * genmatch.c
>>     (parser::parse_operator_list): New member function in parser.
>>     (add_substitute): New function.
>>     (flatten_substitutes): Likewise.
>>     (parser::parse_for): Call add_substitute.
>>     (parser::parse_pattern): Call parser::parse_operator_list.
>>
>> * match-bitwise.pd
>>     (bitwise_ors): New operator-list.
>>     (bitwise_ops): Likewise.
>>     Use bitwise_ors and bitwise_ops in patterns.
>>
>> * match-comparison.pd
>>      (eq_ops): New operator-list.
>>      (cmp_ops): Likewise.
>>      Use cmp_ops and eq_ops in patterns.
>>
>> Thanks,
>> Prathamesh
diff mbox

Patch

Index: gcc/genmatch.c
===================================================================
--- gcc/genmatch.c	(revision 216673)
+++ gcc/genmatch.c	(working copy)
@@ -218,9 +218,10 @@ 
 
 struct user_id : public id_base
 {
-  user_id (const char *id_)
-    : id_base (id_base::USER, id_), substitutes (vNULL) {}
+  user_id (const char *id_, bool is_oper_list_ = false)
+    : id_base (id_base::USER, id_), substitutes (vNULL), is_oper_list (is_oper_list_) {}
   vec<id_base *> substitutes;
+  bool is_oper_list;
 };
 
 template<>
@@ -2419,6 +2420,7 @@ 
   void parse_simplify (source_location, vec<simplify *>&, predicate_id *,
 		       expr *);
   void parse_for (source_location);
+  void parse_operator_list ();
   void parse_if (source_location);
   void parse_predicates (source_location);
 
@@ -2582,8 +2584,14 @@ 
 	   || strcmp  (id, "convert2") == 0)
     fatal_at (id_tok, "expected '?' after conditional operator");
   id_base *op = get_operator (id);
+
   if (!op)
     fatal_at (id_tok, "unknown operator %s", id);
+  
+  user_id *uid;
+  if ((uid = dyn_cast<user_id *> (op)) != 0 && uid->is_oper_list)
+    fatal_at (id_tok, "operator-list '%s' can be used only in for", uid->id);
+
   return op;
 }
 
@@ -2900,6 +2908,20 @@ 
 
 /* Parsing of the outer control structures.  */
 
+bool
+add_substitute (user_id *op, id_base *idb, int& arity) 
+{
+  if (arity == -1)
+    arity = idb->nargs;
+  else if (idb->nargs == -1)
+    ;
+  else if (idb->nargs != arity)
+    return false;
+
+  op->substitutes.safe_push (idb);
+  return true;
+}
+  
 /* Parse a for expression
      for = '(' 'for' <subst>... <pattern> ')'
      subst = <ident> '(' <ident>... ')'  */
@@ -2938,15 +2960,16 @@ 
 	  if (*idb == CONVERT0 || *idb == CONVERT1 || *idb == CONVERT2)
 	    fatal_at (token, "conditional operators cannot be used inside for");
 
-	  if (arity == -1)
-	    arity = idb->nargs;
-	  else if (idb->nargs == -1)
-	    ;
-	  else if (idb->nargs != arity)
-	    fatal_at (token, "operator '%s' with arity %d does not match "
-		      "others with arity %d", oper, idb->nargs, arity);
+	  vec<id_base *> substitutes = vNULL;
+	  if (idb->kind == id_base::USER)
+	    substitutes = (as_a<user_id *> (idb))->substitutes;
+	  else
+	    substitutes.safe_push (idb);
 
-	  op->substitutes.safe_push (idb);
+	  for (unsigned si = 0; si < substitutes.length (); ++si)
+	    if (!add_substitute (op, substitutes[si], arity))
+	      fatal_at (token, "operator '%s' with arity %d does not match "
+			       "others with arity %d", oper, idb->nargs, arity);
 	}
       op->nargs = arity;
       token = expect (CPP_CLOSE_PAREN);
@@ -2997,6 +3020,55 @@ 
     operators->remove_elt (user_ids[i]);
 }
 
+void
+flatten_substitutes (vec<id_base *>& ids, vec<id_base *>& substitutes)
+{
+  user_id *op;
+ 
+  for (unsigned i = 0; i < ids.length (); ++i)
+    if ((op = dyn_cast<user_id *> (ids[i])) != 0)
+      flatten_substitutes (op->substitutes, substitutes);
+    else
+      substitutes.safe_push (ids[i]);
+}
+
+void
+parser::parse_operator_list ()
+{
+  const char *id = get_ident ();
+  user_id *op = new user_id (id, true);
+  vec<const cpp_token *> user_id_tokens = vNULL;
+
+  id_base **slot = operators->find_slot_with_hash (op, op->hashval, INSERT);
+  if (*slot)
+    fatal ("operator %s already defined", id);
+  *slot = op;
+
+  const cpp_token *token;
+  vec<id_base *> ids = vNULL;
+
+  while ((token = peek_ident ()) != 0)
+    {
+      const char *oper = get_ident ();
+      id_base *p = get_operator (oper);
+      if (p == 0)
+	fatal_at (token, "%s is not defined", oper);
+      ids.safe_push (p);
+      user_id_tokens.safe_push (token);
+    }
+
+  if (ids.length () == 0)
+    fatal_at (token, "operator list cannot be empty");
+
+  flatten_substitutes (ids, op->substitutes);
+  int arity = op->substitutes[0]->nargs;
+  for (unsigned i = 1; i < op->substitutes.length (); ++i)
+    if (op->substitutes[i]->nargs != -1 && arity != op->substitutes[i]->nargs)
+      fatal_at (user_id_tokens[i], "operator '%s' with arity %d does not match "
+		       "others with arity %d", op->substitutes[i]->id, op->substitutes[i]->nargs, arity);
+}
+
+
 /* Parse an outer if expression.
      if = '(' 'if' '(' <c-expr> ')' <pattern> ')'  */
 
@@ -3097,6 +3169,8 @@ 
 	fatal_at (token, "define_predicates inside if or for is not supported");
       parse_predicates (token->src_loc);
     }
+  else if (strcmp (id, "define_operator_list") == 0)
+    parse_operator_list ();
   else
     fatal_at (token, "expected %s'simplify', 'match', 'for' or 'if'",
 	      active_ifs.length () == 0 && active_fors.length () == 0
Index: gcc/match-bitwise.pd
===================================================================
--- gcc/match-bitwise.pd	(revision 216673)
+++ gcc/match-bitwise.pd	(working copy)
@@ -17,6 +17,9 @@ 
 along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
+(define_operator_list bitwise_ors bit_ior bit_xor)
+(define_operator_list bitwise_ops bit_and bitwise_ors) 
+
 /* Try to fold (type) X op CST -> (type) (X op ((type-x) CST))
    when profitable.  */
 /* For bitwise binary operations apply operand conversions to the
@@ -23,7 +26,7 @@ 
    binary operation result instead of to the operands.  This allows
    to combine successive conversions and bitwise binary operations.  */
 /* We combine the above two cases by using a conditional convert.  */
-(for bitop (bit_and bit_ior bit_xor)
+(for bitop (bitwise_ops)
  (simplify
   (bitop (convert @0) (convert? @1))
   (if (((TREE_CODE (@1) == INTEGER_CST
@@ -48,7 +51,7 @@ 
    (convert (bitop @0 (convert @1))))))
 
 /* Simplify (A & B) OP0 (C & B) to (A OP0 C) & B. */
-(for bitop (bit_and bit_ior bit_xor)
+(for bitop (bitwise_ops)
  (simplify
   (bitop (bit_and:c @0 @1) (bit_and @2 @1))
   (bit_and (bitop @0 @2) @1)))
@@ -59,7 +62,7 @@ 
   (bit_ior (bit_and @0 @2) (bit_and @1 @2)))
 
 /* Combine successive equal operations with constants.  */
-(for bitop (bit_and bit_ior bit_xor)
+(for bitop (bitwise_ops)
  (simplify
   (bitop (bitop @0 CONSTANT_CLASS_P@1) CONSTANT_CLASS_P@2)
   (bitop @0 (bitop @1 @2))))
@@ -95,7 +98,7 @@ 
  (bit_and @0 (logical_inverted_value @0))
  { build_zero_cst (type); })
 /* X | !X and X ^ !X -> 1, , if X is truth-valued.  */
-(for op (bit_ior bit_xor)
+(for op (bitwise_ors)
  (simplify
   (op truth_valued_p@0 (logical_inverted_value @0))
   { build_one_cst (type); }))
@@ -110,7 +113,7 @@ 
   (bit_and:c @0 (bit_not @0))
   { build_zero_cst (type); })
 /* X | ~X -> ~0,  X ^ ~X -> ~0.  */
-(for bitop (bit_ior bit_xor)
+(for bitop (bitwise_ors)
  (simplify
   (bitop:c @0 (bit_not @0))
   { build_all_ones_cst (type); }))
@@ -118,7 +121,7 @@ 
    Also X != 1 vs. X ^ 1 vs ~X wants canonicalization for truth
    values.  */
 #if 0  /* FIXME.  truth_valued_ssa_name isn't exported either.  */
-(for bitop (bit_and bit_ior bit_xor)
+(for bitop (bitwise_ops)
  /* X & (X == 0) -> 0.  */
  /* X | (X == 0) -> 1.  */
  (simplify
@@ -201,7 +204,7 @@ 
 /* Simple association cases that cancel one operand.  */
 
 /* ((a OP b) & ~a) -> (b & ~a) OP 0  */
-(for bitop (bit_and bit_ior bit_xor)
+(for bitop (bitwise_ops)
  (simplify
   (bit_and:c (bitop:c @0 @1) (bit_not@2 @0))
   (bitop (bit_and @1 @2) { build_zero_cst (type); })))
@@ -264,7 +267,7 @@ 
 /* Similar transform for vector permute.
    ???  Missing an easy way to check if a mask is a reverse
    operation of another mask (most masks are not reversible).  */
-(for bitop (bit_xor bit_ior bit_and)
+(for bitop (bitwise_ops)
   (simplify
     (bitop (vec_perm @1 @2 @0) (vec_perm @3 @4 @0))
     (vec_perm (bitop @1 @3) (bitop @2 @4) @0)))
Index: gcc/match-comparison.pd
===================================================================
--- gcc/match-comparison.pd	(revision 216673)
+++ gcc/match-comparison.pd	(working copy)
@@ -1,5 +1,8 @@ 
 /* From fold_binary.  */
 
+(define_operator_list eq_ops eq ne)
+(define_operator_list cmp_ops eq_ops lt le gt ge) 
+
 /* On GIMPLE bool != 0 is simply the canonical way to express a
    condition in COND_EXPRs and GIMPLE_CONDs.
    ???  Of course for assignments we still may want to strip those...  */
@@ -15,7 +18,7 @@ 
     @0)))
 
 /* Distribute operations in equality compares.  */
-(for op (eq ne)
+(for op (eq_ops)
  /* -exp op CST is exp op -CST.  */
  (simplify
   /* ??? fix fold-const to use negate_expr_p  */
@@ -29,7 +32,7 @@ 
 /* From fold_comparison, in the order of transforms in there.  */
 
 /* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1.  */
-(for cmp (lt le eq ge gt ne)
+(for cmp (cmp_ops)
  (for op (plus minus)
   (simplify
    (cmp (op @0 INTEGER_CST@1) INTEGER_CST@2)
@@ -47,7 +50,7 @@ 
    is undefined for the type, but performing it here badly interacts
    with the transformation in fold_cond_expr_with_comparison which
    attempts to synthetize ABS_EXPR.  */
-(for cmp (eq ne)
+(for cmp (eq_ops)
  (simplify
   (cmp (minus @0 @1) integer_zerop)
   (cmp @0 @1)))
@@ -65,7 +68,7 @@ 
 #endif
 
 /* Simplify X * C1 CMP 0 to X CMP 0 if C1 is not zero.  */
-(for op (lt le eq ne ge gt)
+(for op (cmp_ops)
   (simplify
     (op (mult @0 INTEGER_CST@1) integer_zerop@2)
     /* In fold-const.c we have this and the following pattern
@@ -131,13 +134,13 @@ 
 #endif
 
 /* Fold ~X op ~Y as Y op X.  */
-(for cmp (lt le eq ge gt ne)
+(for cmp (cmp_ops)
  (simplify
   (cmp (bit_not @0) (bit_not @1))
   (cmp @1 @0)))
 
 /* Fold ~X op C as X op' ~C, where op' is the swapped comparison.  */
-(for cmp (lt le eq ge gt ne)
+(for cmp (cmp_ops)
  (simplify
   (cmp (bit_not @0) @1)
   /* ???  (for cst in INTEGER_CST VECTOR_CST) is not supported yet.  */