diff mbox

[match-and-simplify] operator-lists in expression

Message ID CAJXstsCvBzsSiofPpGFr24+CSn0fp1ZN++B90wN54gUqS2wULw@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni Nov. 10, 2014, 1:39 p.m. UTC 
Hi,
  This patch adds support for operator-lists to be used in expression.

I reuse operator-list as the iterator. This is not really valid since
user-defined operator-lists cannot be iterator in 'for', but it was
convenient to reuse operator-list as a 'for' iterator
and lower_for doesn't care about that.
eg:
(define_operator_list  list1 plus minus)

(simplify
  (list1 @x integer_zerop)
  (non_lvalue @x))

is wrapped into 'for' as: (lower_operator_list):
(for list1 (plus minus)
  (simplify
    (list1 @x integer_zerop)
    (non_lvalue @x)))

this is not really valid since we reject list1 to be used as iterator if
it were written by user.

Is this okay or should I introduce an explicit temporary iterator ?
so it gets lowered to something like:
(for tmp1 (list1)
  (simplify
    (tmp1 @x integer_zerop)
    (non_lvalue @x)))

* genmatch.c
  (fatal_at): New overloaded function.
  (simplify::oper_lists): New member.
  (simplify::simplify): Add default argument.
  (lower_commutative): Adjust call to simplify::simplify.
  (lower_opt_convert): Likewise.
  (lower_operator_list): New function.
  (lower): Call lower_operator_list.
  (parser::parsing_for_p): New member function.
  (parser::oper_lists): New member.
  (parser::parse_operation): Check for operator-list.
  (parser::parse_c_expr): Likewise.
  (parser::parse_simplify): Reset parser::oper_lists.
    Adjust call to simplify::simplify.
  (parser::parser): Initialize parser::oper_lists.

* match-builtin.pd:
  Adjust patten to use SQRTs and POWs.

Thanks,
Prathamesh

Comments

Richard Biener Nov. 11, 2014, 8:55 a.m. UTC | #1 
On Mon, Nov 10, 2014 at 2:39 PM, Prathamesh Kulkarni
<bilbotheelffriend@gmail.com> wrote:
> Hi,
>   This patch adds support for operator-lists to be used in expression.
>
> I reuse operator-list as the iterator. This is not really valid since
> user-defined operator-lists cannot be iterator in 'for', but it was
> convenient to reuse operator-list as a 'for' iterator
> and lower_for doesn't care about that.
> eg:
> (define_operator_list  list1 plus minus)
>
> (simplify
>   (list1 @x integer_zerop)
>   (non_lvalue @x))
>
> is wrapped into 'for' as: (lower_operator_list):
> (for list1 (plus minus)
>   (simplify
>     (list1 @x integer_zerop)
>     (non_lvalue @x)))
>
> this is not really valid since we reject list1 to be used as iterator if
> it were written by user.
>
> Is this okay or should I introduce an explicit temporary iterator ?

No, it's ok to re-use it.

I think you should get rid of the extra lowering step and instead
in parse_simplify create the extra for directly when building
a simplify (the multiple simplfy buildings really ask for factoring
it out to a method in the parser class which has access to
active_fors, active_ifs and friends).

Also you use a vector to store operator_lists - this will gobble
up duplicates.  It's probably better to use a pointer_hash <user_id *>
for this.

Thanks for continuing to work on this!

Richard.

> so it gets lowered to something like:
> (for tmp1 (list1)
>   (simplify
>     (tmp1 @x integer_zerop)
>     (non_lvalue @x)))
>
> * genmatch.c
>   (fatal_at): New overloaded function.
>   (simplify::oper_lists): New member.
>   (simplify::simplify): Add default argument.
>   (lower_commutative): Adjust call to simplify::simplify.
>   (lower_opt_convert): Likewise.
>   (lower_operator_list): New function.
>   (lower): Call lower_operator_list.
>   (parser::parsing_for_p): New member function.
>   (parser::oper_lists): New member.
>   (parser::parse_operation): Check for operator-list.
>   (parser::parse_c_expr): Likewise.
>   (parser::parse_simplify): Reset parser::oper_lists.
>     Adjust call to simplify::simplify.
>   (parser::parser): Initialize parser::oper_lists.
>
> * match-builtin.pd:
>   Adjust patten to use SQRTs and POWs.
>
> Thanks,
> Prathamesh
Richard Biener Dec. 1, 2014, 3:12 p.m. UTC | #2 
On Tue, Nov 11, 2014 at 9:55 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Mon, Nov 10, 2014 at 2:39 PM, Prathamesh Kulkarni
> <bilbotheelffriend@gmail.com> wrote:
>> Hi,
>>   This patch adds support for operator-lists to be used in expression.
>>
>> I reuse operator-list as the iterator. This is not really valid since
>> user-defined operator-lists cannot be iterator in 'for', but it was
>> convenient to reuse operator-list as a 'for' iterator
>> and lower_for doesn't care about that.
>> eg:
>> (define_operator_list  list1 plus minus)
>>
>> (simplify
>>   (list1 @x integer_zerop)
>>   (non_lvalue @x))
>>
>> is wrapped into 'for' as: (lower_operator_list):
>> (for list1 (plus minus)
>>   (simplify
>>     (list1 @x integer_zerop)
>>     (non_lvalue @x)))
>>
>> this is not really valid since we reject list1 to be used as iterator if
>> it were written by user.
>>
>> Is this okay or should I introduce an explicit temporary iterator ?
>
> No, it's ok to re-use it.
>
> I think you should get rid of the extra lowering step and instead
> in parse_simplify create the extra for directly when building
> a simplify (the multiple simplfy buildings really ask for factoring
> it out to a method in the parser class which has access to
> active_fors, active_ifs and friends).
>
> Also you use a vector to store operator_lists - this will gobble
> up duplicates.  It's probably better to use a pointer_hash <user_id *>
> for this.
>
> Thanks for continuing to work on this!

I have picked this up now and did the suggested changes.  I've also
removed the restriction on the use in for as we have one case
where that's useful in match-builtins.pd already.  I have for now
restricted the operator lists to combine a little more than for
regular fors - namely I require the same number of operators.

I am testing the following patch on trunk and hope to commit it there
tomorrow and then bring it back to the branch via a merge.

Bootstrap running on x86_64-unknown-linux-gnu (testing not neccessary
on trunk as the generated files are unchanged).  Bootstrap / test on
the branch running as well.

Richard.

2014-12-01  Richard Biener  <rguenther@suse.de>
        Prathamesh Kulkarni  <bilbotheelffriend@gmail.com>

        * genmatch.c: Include hash-set.h.
        (fatal_at): Add source_location overload.
        (parser::record_operlist): New method.
        (parser::push_simplify): Likewise.
        (parser::oper_lists_set): New member.
        (parser::oper_lists): Likewise.
        (parser::parse_operation): Record seen operator list references.
        (parser::parse_c_expr): Likewise.
        (parser::parse_simplify): Init oper_lists_set and oper_lists
        and use push_simplify.
        (parser::parser): Init oper_lists_set and oper_lists.


> Richard.
>
>> so it gets lowered to something like:
>> (for tmp1 (list1)
>>   (simplify
>>     (tmp1 @x integer_zerop)
>>     (non_lvalue @x)))
>>
>> * genmatch.c
>>   (fatal_at): New overloaded function.
>>   (simplify::oper_lists): New member.
>>   (simplify::simplify): Add default argument.
>>   (lower_commutative): Adjust call to simplify::simplify.
>>   (lower_opt_convert): Likewise.
>>   (lower_operator_list): New function.
>>   (lower): Call lower_operator_list.
>>   (parser::parsing_for_p): New member function.
>>   (parser::oper_lists): New member.
>>   (parser::parse_operation): Check for operator-list.
>>   (parser::parse_c_expr): Likewise.
>>   (parser::parse_simplify): Reset parser::oper_lists.
>>     Adjust call to simplify::simplify.
>>   (parser::parser): Initialize parser::oper_lists.
>>
>> * match-builtin.pd:
>>   Adjust patten to use SQRTs and POWs.
>>
>> Thanks,
>> Prathamesh
diff mbox

Patch

Index: gcc/genmatch.c
===================================================================
--- gcc/genmatch.c	(revision 217284)
+++ gcc/genmatch.c	(working copy)
@@ -110,6 +110,18 @@ 
 #if GCC_VERSION >= 4001
 __attribute__((format (printf, 2, 3)))
 #endif
+fatal_at (source_location loc, const char *msg, ...)
+{
+  va_list ap;
+  va_start (ap, msg);
+  error_cb (NULL, CPP_DL_FATAL, 0, loc, 0, msg, &ap);
+  va_end (ap);
+}
+
+static void
+#if GCC_VERSION >= 4001
+__attribute__((format (printf, 2, 3)))
+#endif
 warning_at (const cpp_token *tk, const char *msg, ...)
 {
   va_list ap;
@@ -549,11 +561,11 @@ 
   simplify (operand *match_, source_location match_location_,
 	    struct operand *result_, source_location result_location_,
 	    vec<if_or_with> ifexpr_vec_, vec<vec<user_id *> > for_vec_,
-	    cid_map_t *capture_ids_)
+	    cid_map_t *capture_ids_, vec<user_id *> oper_lists_ = vNULL)
       : match (match_), match_location (match_location_),
       result (result_), result_location (result_location_),
       ifexpr_vec (ifexpr_vec_), for_vec (for_vec_),
-      capture_ids (capture_ids_), capture_max (capture_ids_->elements () - 1) {}
+      capture_ids (capture_ids_), capture_max (capture_ids_->elements () - 1), oper_lists (oper_lists_) {}
 
   /* The expression that is matched against the GENERIC or GIMPLE IL.  */
   operand *match;
@@ -572,6 +584,8 @@ 
   /* A map of capture identifiers to indexes.  */
   cid_map_t *capture_ids;
   int capture_max;
+  /* collected operator-list used in expression */
+  vec<user_id *> oper_lists;
 };
 
 /* Debugging routines for dumping the AST.  */
@@ -721,7 +735,7 @@ 
     {
       simplify *ns = new simplify (matchers[i], s->match_location,
 				   s->result, s->result_location, s->ifexpr_vec,
-				   s->for_vec, s->capture_ids);
+				   s->for_vec, s->capture_ids, s->oper_lists);
       simplifiers.safe_push (ns);
     }
 }
@@ -837,7 +851,7 @@ 
     {
       simplify *ns = new simplify (matchers[i], s->match_location,
 				   s->result, s->result_location, s->ifexpr_vec,
-				   s->for_vec, s->capture_ids);
+				   s->for_vec, s->capture_ids, s->oper_lists);
       simplifiers.safe_push (ns);
     }
 }
@@ -934,6 +948,38 @@ 
     simplifiers.safe_push (worklist[i]);
 }
 
+static void
+lower_operator_list (simplify *s, vec<simplify *>& simplifiers)
+{
+  vec<user_id *> oper_lists = s->oper_lists;
+  if (oper_lists == vNULL)
+    {
+      simplifiers.safe_push (s);
+      return;
+    } 
+
+  unsigned min = oper_lists[0]->substitutes.length ();
+  for (unsigned i = 1; i < oper_lists.length (); ++i)
+    if (min > oper_lists[i]->substitutes.length ())
+      min = oper_lists[i]->substitutes.length ();
+
+  for (unsigned i = 0; i < oper_lists.length (); ++i)
+    if (oper_lists[i]->substitutes.length () % min != 0)
+      fatal_at (s->match_location, "All user-defined identifiers must have a multiple number "
+				   "of operator substittions of the smallest number of substitutions");
+
+  vec< vec<user_id *> > for_vec = vNULL;
+  for_vec.safe_push (oper_lists);
+
+  simplify ns (s->match, s->match_location,
+	       s->result, s->result_location,
+	       s->ifexpr_vec, for_vec,
+	       s->capture_ids);
+
+  lower_for (&ns, simplifiers);
+}
+
+
 /* Lower the AST for everything in SIMPLIFIERS.  */
 
 static void
@@ -947,14 +993,15 @@ 
   for (unsigned i = 0; i < out_simplifiers0.length (); ++i)
     lower_commutative (out_simplifiers0[i], out_simplifiers1);
 
+  auto_vec<simplify *> out_simplifiers2;
+  for (unsigned i = 0;  i < out_simplifiers1.length (); ++i)
+    lower_operator_list (out_simplifiers1[i], out_simplifiers2);
+
   simplifiers.truncate (0);
-  for (unsigned i = 0; i < out_simplifiers1.length (); ++i)
-    lower_for (out_simplifiers1[i], simplifiers);
+  for (unsigned i = 0; i < out_simplifiers2.length (); ++i)
+    lower_for (out_simplifiers2[i], simplifiers);
 }
 
-
-
-
 /* The decision tree built for generating GIMPLE and GENERIC pattern
    matching code.  It represents the 'match' expression of all
    simplifies and has those as its leafs.  */
@@ -2498,10 +2545,13 @@ 
   void parse_if (source_location);
   void parse_predicates (source_location);
   void parse_operator_list (source_location);
+  
+  bool parsing_for_p () { return active_fors.length () != 0; }
 
   cpp_reader *r;
   vec<if_or_with> active_ifs;
   vec<vec<user_id *> > active_fors;
+  vec<user_id *> oper_lists;
 
   cid_map_t *capture_ids;
 
@@ -2669,8 +2719,11 @@ 
 
   user_id *p = dyn_cast<user_id *> (op);
   if (p && p->is_oper_list)
-    fatal_at (id_tok, "operator-list not allowed in expression");
-
+    {
+      if (parsing_for_p ()) 
+	fatal_at (id_tok, "operator-list not allowed in expression enclosed in 'for'");
+      oper_lists.safe_push (p);
+    }
   return op;
 }
 
@@ -2807,8 +2860,13 @@ 
 
       /* If this is possibly a user-defined identifier mark it used.  */
       if (token->type == CPP_NAME)
-	get_operator ((const char *)CPP_HASHNODE
-		        (token->val.node.node)->ident.str);
+	{
+	  id_base *idb = get_operator ((const char *)CPP_HASHNODE
+				      (token->val.node.node)->ident.str);
+	  user_id *p;
+	  if (idb && (p = dyn_cast<user_id *> (idb)) && p->is_oper_list)
+	    oper_lists.safe_push (p);
+	}
 
       /* Record the token.  */
       code.safe_push (*token);
@@ -2893,6 +2951,8 @@ 
 {
   /* Reset the capture map.  */
   capture_ids = new cid_map_t;
+  /* Reset oper_lists */
+  oper_lists = vNULL;
 
   const cpp_token *loc = peek ();
   parsing_match_operand = true;
@@ -2915,7 +2975,7 @@ 
       simplifiers.safe_push
 	(new simplify (match, match_location, result, token->src_loc,
 		       active_ifs.copy (), active_fors.copy (),
-		       capture_ids));
+		       capture_ids, oper_lists.copy ()));
       return;
     }
 
@@ -2941,7 +3001,7 @@ 
 		  simplifiers.safe_push
 		      (new simplify (match, match_location, result,
 				     paren_loc, active_ifs.copy (),
-				     active_fors.copy (), capture_ids));
+				     active_fors.copy (), capture_ids, oper_lists.copy ()));
 		}
 	    }
 	  else if (peek_ident ("with"))
@@ -2960,7 +3020,7 @@ 
 	      simplifiers.safe_push
 		  (new simplify (match, match_location, op,
 				 token->src_loc, active_ifs.copy (),
-				 active_fors.copy (), capture_ids));
+				 active_fors.copy (), capture_ids, oper_lists.copy ()));
 	      eat_token (CPP_CLOSE_PAREN);
 	      /* A "default" result closes the enclosing scope.  */
 	      if (active_ifs.length () > active_ifs_len)
@@ -2992,7 +3052,7 @@ 
 	      (new simplify (match, match_location,
 			     matcher ? result : parse_op (),
 			     token->src_loc, active_ifs.copy (),
-			     active_fors.copy (), capture_ids));
+			     active_fors.copy (), capture_ids, oper_lists.copy ()));
 	  /* A "default" result closes the enclosing scope.  */
 	  if (active_ifs.length () > active_ifs_len)
 	    {
@@ -3286,6 +3346,7 @@ 
   active_ifs = vNULL;
   active_fors = vNULL;
   simplifiers = vNULL;
+  oper_lists = vNULL;
   user_predicates = vNULL;
   parsing_match_operand = false;
 
Index: gcc/match-builtin.pd
===================================================================
--- gcc/match-builtin.pd	(revision 217284)
+++ gcc/match-builtin.pd	(working copy)
@@ -147,8 +147,6 @@ 
       dconstroot = real_value_truncate (TYPE_MODE (type), dconstroot); }
     (POW @0 { build_real (type, dconstroot); }))))
  /* Optimize sqrt(pow(x,y)) = pow(|x|,y*0.5).  */
- (for SQRT (BUILT_IN_SQRTF BUILT_IN_SQRT BUILT_IN_SQRTL)
-      POW (BUILT_IN_POWF BUILT_IN_POW BUILT_IN_POWL)
   (simplify
-   (SQRT (POW @0 @1))
-   (POW (abs @0) (mult @1 { build_real (TREE_TYPE (@1), dconsthalf); })))))
+   (SQRTs (POWs @0 @1))
+   (POWs (abs @0) (mult @1 { build_real (TREE_TYPE (@1), dconsthalf); }))))