This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [GSoC] replacing op in c_expr


On Sat, Aug 16, 2014 at 3:46 PM, Prathamesh Kulkarni
<bilbotheelffriend@gmail.com> wrote:
> On Mon, Aug 11, 2014 at 4:58 PM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Sun, Aug 10, 2014 at 11:17 PM, Prathamesh Kulkarni
>> <bilbotheelffriend@gmail.com> wrote:
>>> On Mon, Aug 4, 2014 at 2:13 PM, Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>> On Sun, Aug 3, 2014 at 6:58 PM, Prathamesh Kulkarni
>>>> <bilbotheelffriend@gmail.com> wrote:
>>>>> On Tue, Jul 29, 2014 at 4:29 PM, Richard Biener
>>>>> <richard.guenther@gmail.com> wrote:
>>>>>> On Mon, Jul 28, 2014 at 10:02 PM, Prathamesh Kulkarni
>>>>>> <bilbotheelffriend@gmail.com> wrote:
>>>>>>> I am having few issues replacing op in c_expr.
>>>>>>> I thought of following possibilities:
>>>>>>>
>>>>>>> a) create a new vec<cpp_token> vector new_code.
>>>>>>> for each token in code
>>>>>>>   {
>>>>>>>     if token.type is not CPP_NAME
>>>>>>>       new_code.safe_push (token);
>>>>>>>     else
>>>>>>>      {
>>>>>>>         cpp_token new_token =
>>>>>>>             ??? create new token of type CPP_NAME
>>>>>>>                   with contents as name of operator ???
>>>>>>>      }
>>>>>>>   }
>>>>>>>
>>>>>>> I tried to go this way, but am stuck with creating a new token type.
>>>>>>> i started by:
>>>>>>> cpp_token new_token = token;  // get same attrs as token.
>>>>>>> CPP_HASHNODE (new_token.val.node.node)->ident.str = name of operator.
>>>>>>> CPP_HASHNODE (new_token.val.node.node)->ident.len = len of operator name.
>>>>>>> name of operator is obtained from opers[i] in parse_for.
>>>>>>>
>>>>>>> however this does not work because I guess
>>>>>>>  new_token = token, shallow copies
>>>>>>> the token (default assignment operator, i didn't find an overloaded version).
>>>>>>>
>>>>>>> b) create new struct c_expr_elem and use
>>>>>>> vec<c_expr_elem> code, instead of vec<cpp_token> code;
>>>>>>>
>>>>>>> sth like:
>>>>>>> struct c_expr_elem
>>>>>>> {
>>>>>>>    enum c_expr_elem_type { ID, TOKEN };
>>>>>>>    enum c_expr_elem_type type;
>>>>>>>
>>>>>>>    union {
>>>>>>>      cpp_token token;
>>>>>>>      const char *id;
>>>>>>>    };
>>>>>>> };
>>>>>>>
>>>>>>> while replacing op, compare token with op, and if it matches,
>>>>>>> create a new c_expr_elem with type = ID, and id = name of operator.
>>>>>>> This shall probably work, but shall require many changes to other parts
>>>>>>> since we change c_expr::code.
>>>>>>>
>>>>>>> I would like to hear any other suggestions.
>>>>>>
>>>>>> Together with the vector of tokens recorded at parse_c_expr time
>>>>>> record a vector of token mappings (op -> plus, op2 -> ...) and do
>>>>>> the replacement at code-generation time where we also special-case
>>>>>> captures.
>>>>>>
>>>>>> Yeah, it's a but unfortunate that c_expr parsing is done the way it
>>>>>> is done....
>>>>> Thanks. I guess we would require a multi-map for this since there can
>>>>> be many operators
>>>>> (op -> [plus, minus], op2 -> [negate]) ?
>>>>
>>>> Well, it would be enough to attach the mapping to c_expr()s after the
>>>> AST lowering when there is at most one?  Because obviously
>>>> code-generation cannot know which to replace.
>>>>
>>>>> Unfortunately, I somehow seem to have missed your response and ended up with a
>>>>> hackish way of doing it, although it works. I will soon change that to
>>>>> use token mappings.
>>>>>
>>>>> I mostly followed b), except i made it sub-class of cpp_token, so the
>>>>> other code using c_expr::code
>>>>> (outline_c_expr, c_expr::gen_transform) did not require changes except
>>>>> for special-casing op.
>>>>
>>>> Indeed not too ugly.  Still at the point where you replace in the for()
>>>> processing
>>>>
>>>> -             operand *result_op = replace_id (s->result, user_id, opers[i]);
>>>> +
>>>> +             operand *result_op;
>>>> +             if (is_a<c_expr *> (s->result))
>>>> +               result_op = replace_op_in_c_expr (s->result, user_id, opers[i]);
>>>> +             else
>>>> +               result_op = replace_id (s->result, user_id, opers[i]);
>>>> +
>>>> +
>>>>
>>>> it should be "easy" to attach a replacemet vector/map to the c_expr
>>>> and use that duing code-generation.
>>>>
>>>> Note that sub-expressions can also be c_exprs, thus
>>>>
>>>> (match-and-simplify
>>>>    (....)
>>>>    (plus { ... } @2))
>>>>
>>>> I don't think your patch covers that.  That is, you should add
>>>> c_expr handing to replace_id instead.
>>> Thanks, this patch covers that case.
>>> For now, I have still kept the old way, since the change was one-liner.
>>> I will change it after I am done with conditional convert
>>
>> I'll wait for that - the patch introduces extra warnings which will break
>> bootstrap.
>>
> Hi,
>     This patch replaces op in c_expr, by using vector in c_expr to record
> <user_id, operator> mapping.
> Sorry for late response.
>
> I needed to clone c_expr, so added clone member function to operand hierarchy.
> Ideally it should be a pure member function (= 0) in operand, however
> for simplicity I have
> put gcc_unreachable (), since I only want it used for c_expr (not
> required so far for other classes).
> Is that okay for now ? Eventually I will implement clone for other classes...

Hmm, I wonder why you didn't go with the simpler approach of simply
copying the replacement vector in replace_id?  Like the attached
which I have applied now.

I noted we lack any exercising patterns for this so I added one for
comparison folding also noting another deficiency (we can't
"compute" new operators - somewhat by design as we statically
determine them to simplify further).  I have to think about this
(also in the context of manual simplifiers).

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

        * match-comparison.pd: New file.
        * match.pd: Include match-comparison.pd.
        * genmatch.c (c_expr): Add vec<id_tab> member.
        (replace_id): Handle replacing in c_exprs.
        (c_expr::gen_transform): Handle replacing identifiers.
        (outline_c_exprs): Likewise.
        (parse_for): Replace in if-exprs.



Richard.

> * genmatch.c (operand): New member function clone.
>      (c_expr): Override clone.
>      (c_expr): New struct id_tab.
>      (c_expr): New member ids.
>      (simplify::simplify): Adjust to clone c_expr.
>      (replace_id): New default parameter replace_c_expr.
>      (c_expr::gen_transform): Adjust to replace user-defined
> identifier in c_expr.
>      (outline_c_expr): Likewise.
>      (parse_for): Likewise.
>
> Thanks,
> Prathamesh
>
>> Thanks,
>> Richard.
>>
>>> * genmatch.c (c_expr::elem): New struct.
>>>      (c_expr::code): Change type to vec<c_expr::elem>.
>>>      (replace_op_in_c_expr): New function.
>>>      (replace_id): Call replace_op_in_c_expr.
>>>      (c_expr::gen_transform): Adjust to changes in c_expr.
>>>      (outline_c_expr): Likewise.
>>>      (parse_c_expr): Likewise.
>>>      (parse_for): Call replace_op_in_c_expr.
>>> Thanks,
>>> Prathamesh
>>>>
>>>> Richard.
>>>>
>>>>> * genmatch.c (c_expr::elem): New struct.
>>>>>      (c_expr::code): Change type to vec<c_expr::elem>.
>>>>>      (replace_op_in_c_expr): New function.
>>>>>      (c_expr::gen_transform): Adjust to changes in c_expr.
>>>>>      (outline_c_expr): Likewise.
>>>>>      (parse_c_expr): Likewise.
>>>>>      (parse_for): Call replace_op_in_c_expr.
>>>>>
>>>>> Thanks,
>>>>> Prathamesh
>>>>>
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>> Thanks,
>>>>>>> Prathamesh.
Index: gcc/genmatch.c
===================================================================
--- gcc/genmatch.c	(revision 214018)
+++ gcc/genmatch.c	(working copy)
@@ -251,13 +251,24 @@ struct expr : public operand
 
 struct c_expr : public operand
 {
-  c_expr (cpp_reader *r_, vec<cpp_token> code_, unsigned nr_stmts_)
+  struct id_tab {
+    const char *id;
+    const char *oper;
+
+    id_tab (const char *id_, const char *oper_): id (id_), oper (oper_) {}
+  };
+
+
+  c_expr (cpp_reader *r_, vec<cpp_token> code_, unsigned nr_stmts_,
+	  vec<id_tab> ids_ = vNULL)
     : operand (OP_C_EXPR), r (r_), code (code_),
-      nr_stmts (nr_stmts_), fname (NULL) {}
+      nr_stmts (nr_stmts_), fname (NULL), ids (ids_) {}
   cpp_reader *r;
   vec<cpp_token> code;
   unsigned nr_stmts;
   char *fname;
+  vec<id_tab> ids;
+
   virtual void gen_transform (FILE *f, const char *, bool, int);
 };
 
@@ -771,6 +782,14 @@ replace_id (operand *o, const char *user
       return nc;
     }
 
+  if (c_expr *ce = dyn_cast<c_expr *> (o))
+    {
+      id_base *idb = get_operator (oper);
+      vec<c_expr::id_tab> ids = ce->ids.copy ();
+      ids.safe_push (c_expr::id_tab (user_id, idb->id));
+      return new c_expr (ce->r, ce->code, ce->nr_stmts, ids);
+    }
+
   if (o->type != operand::OP_EXPR)
     return o;
 
@@ -912,6 +931,22 @@ c_expr::gen_transform (FILE *f, const ch
       if (token->flags & PREV_WHITE)
 	fputc (' ', f);
 
+      if (token->type == CPP_NAME)
+	{
+	  const char *id = (const char *) NODE_NAME (token->val.node.node);
+	  unsigned j;
+	  for (j = 0; j < ids.length (); ++j)
+	    {
+	    if (strcmp (id, ids[j].id) == 0)
+	      {
+		fprintf (f, "%s", ids[j].oper);
+		break;
+	      }
+	    }
+	  if (j < ids.length ())
+	    continue;
+	}
+
       /* Output the token as string.  */
       char *tk = (char *)cpp_token_as_text (r, token);
       fputs (tk, f);
@@ -1912,6 +1947,24 @@ outline_c_exprs (FILE *f, struct operand
 	      if (token->flags & PREV_WHITE)
 		fputc (' ', f);
 
+	      if (token->type == CPP_NAME)
+		{
+		  const char *id = (const char *) NODE_NAME (token->val.node.node);
+		  unsigned j;
+
+		  for (j = 0; j < e->ids.length (); ++j)
+		    {
+		      if (strcmp (id, e->ids[j].id) == 0)
+			{
+			  fprintf (f, "%s", e->ids[j].oper);
+			  break;
+			}
+		    }
+
+		  if (j < e->ids.length ())
+		    continue;
+		}
+
 	      /* Output the token as string.  */
 	      char *tk = (char *)cpp_token_as_text (e->r, token);
 	      fputs (tk, f);
@@ -2318,7 +2371,6 @@ parse_for (cpp_reader *r, source_locatio
   if (token->type == CPP_CLOSE_PAREN)
     fatal_at (token, "no pattern defined in for");
 
-
   while (1)
     {
       const cpp_token *token = peek (r);
@@ -2327,17 +2379,22 @@ parse_for (cpp_reader *r, source_locatio
       
       vec<simplify *> for_simplifiers = vNULL;
       parse_pattern (r, for_simplifiers);
-      
+
       for (unsigned i = 0; i < opers.length (); ++i)
 	{
+	  id_base *idb = get_operator (opers[i]);	      
+	  gcc_assert (idb);
+
 	  for (unsigned j = 0; j < for_simplifiers.length (); ++j)
 	    {
 	      simplify *s = for_simplifiers[j];
 	      operand *match_op = replace_id (s->match, user_id, opers[i]);
 	      operand *result_op = replace_id (s->result, user_id, opers[i]);
-
+	      vec<operand *> ifexpr_vec = vNULL;
+	      for (unsigned j = 0; j < s->ifexpr_vec.length (); ++j)
+		ifexpr_vec.safe_push (replace_id (s->ifexpr_vec[j], user_id, opers[i]));
 	      simplify *ns = new simplify (s->name, match_op, s->match_location,
-					   result_op, s->result_location, s->ifexpr_vec);
+					   result_op, s->result_location, ifexpr_vec);
 
 	      simplifiers.safe_push (ns);
 	    }
Index: gcc/match.pd
===================================================================
--- gcc/match.pd	(revision 214018)
+++ gcc/match.pd	(working copy)
@@ -112,6 +112,7 @@ along with GCC; see the file COPYING3.
 #include "match-rotate.pd"
 #include "match-builtin.pd"
 #include "match-constant-folding.pd"
+#include "match-comparison.pd"
 
 /* ????s
 
Index: gcc/match-comparison.pd
===================================================================
--- gcc/match-comparison.pd	(revision 0)
+++ gcc/match-comparison.pd	(working copy)
@@ -0,0 +1,17 @@
+(for op in eq ne
+  /* Simplify X * C1 CMP 0 to X CMP 0 if C1 is not zero.  */
+  (simplify
+    (op (mult @0 INTEGER_CST_P@1) integer_zerop@2)
+    /* In fold-const.c we have this and the following patterns
+       combined because there we can "compute" the operator
+       to use by using swap_tree_comparison.  */
+    (if (tree_int_cst_sgn (@1) > 0))
+    (op @0 @2))
+  (simplify
+    (op (mult @0 INTEGER_CST_P@1) integer_zerop@2)
+    (if (tree_int_cst_sgn (@1) < 0 && op == EQ_EXPR))
+    (ne @0 @2))
+  (simplify
+    (op (mult @0 INTEGER_CST_P@1) integer_zerop@2)
+    (if (tree_int_cst_sgn (@1) < 0 && op == NE_EXPR))
+    (eq @0 @2)))

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]