Re: [GSoC] symbol to denote multiple operators

[Prathamesh Kulkarni <bilbotheelffriend at gmail dot com>]

On Mon, Jul 14, 2014 at 5:36 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Fri, Jul 11, 2014 at 4:59 PM, Prathamesh Kulkarni
> <bilbotheelffriend@gmail.com> wrote:
>> On 7/11/14, Richard Biener <richard.guenther@gmail.com> wrote:
>>> On Thu, Jul 10, 2014 at 8:05 PM, Prathamesh Kulkarni
>>> <bilbotheelffriend@gmail.com> wrote:
>>>> Hi,
>>>>    I have attempted to add syntax for symbol to denote multiple
>>>> operators.
>>>>
>>>> I tried it with few bogus patterns and it appears to work.... hopefully
>>>> -:)
>>>> eg: (bogus pattern):
>>>> (for op in plus minus
>>>>   (match_and_simplify
>>>>     (op @0 @1)
>>>>     (op @0 @0)))
>>>>
>>>> generates following patterns:
>>>> (plus @0 @1) -> (plus @0 @0)  // simplify_0
>>>> (plus @0 @1) -> (mult @0 @0)  // simplify_1
>>>> (mult @0 @1) -> (plus @0 @0)  // simplify_2
>>>> (mult @0 @1) -> (mult @0 @0)  // simplify_3
>>>
>>> s/mult/minus/?  I think it should only generate
>>>
>>>  (plus @0 @1) -> (plus @0 @0)
>>>  (minus @0 @1) -> (minus @0 @0)
>>>
>>> to get the what you write we should need to write
>>>
>>>  (for op1 in plus minus
>>>    (for op2 in plus minus
>>>      (match_and_simplify
>>>         (op1 @0 @1)
>>>         (op2 @0 @0))))
>>>
>>>> root (0xab6b10), 0, 2
>>>> |--(PLUS_EXPR) (0xab6b30), 1, 1
>>>> |----true (0xab6ba0), 2, 1
>>>> |------true (0xab6c10), 3, 2
>>>> |--------simplify_0 { 0xab6ba0, 0xab6c10, (nil), (nil),  }  (0xab6c80), 4,
>>>> 0
>>>> |--------simplify_1 { 0xab6ba0, 0xab6c10, (nil), (nil),  }  (0xab6d40), 4,
>>>> 0
>>>> |--(MULT_EXPR) (0xab6d00), 1, 1
>>>> |----true (0xab6d90), 2, 1
>>>> |------true (0xab6e00), 3, 2
>>>> |--------simplify_2 { 0xab6d90, 0xab6e00, (nil), (nil),  }  (0xab6e70), 4,
>>>> 0
>>>> |--------simplify_3 { 0xab6d90, 0xab6e00, (nil), (nil),  }  (0xab6f30), 4,
>>>> 0
>>>>
>>>> * Changes to rest of the code:
>>>> a) commutating patterns was interfering with this, because
>>>> parse_match_and_simplify, immediately commutated match operand. Symbol
>>>> should be replaced by operators before commutating. This required
>>>> adjusting simplify (back to operand *match), and commutating is done
>>>> in new function lower_commutative. Ideally this should be done during
>>>> insertion in decision tree ?
>>>
>>> Yes, commutating should be done by inserting a pattern multiple
>>> times with adjusted AST walk order.
>>>
>>>> b) adjustments required to e_operation constructor, so it doesn't
>>>> call fatal, when it does not find id to be in the hash table.
>>>
>>> Eventually just temporarily insert a new operator in the hash table
>>> when parsing comes along a (for OP ...?  That is, add a new kind,
>>> USER and just re-use base->id?
>>>
>>>> * Caveats
>>>> a) new e_operation constructor taking id_base * argument. Not sure
>>>> if that's required.
>>>> b) e_operation::user_id denotes user-defined identifier (<opname>),
>>>> a rather apologetic name ...
>>>> c) Similar to commutate(), replace_user_id() does not clone AST's.
>>>> So we have multiple AST's sharing same nodes.
>>>
>>> I wonder if we want to parse the 'for' into an AST node instead,
>>> thus not expand it on-the-fly.  Applying could then happen during
>>> DT insertion.  Or as a separate lowering like you introduce
>>> lower_commutative - that looks  cleaner.
>>>
>>> Or if we can simply parse the match-and-simplify multiple times, with
>>> the for op replaces, using _cpp_backup_tokens.  Ok, no - that
>>> sounds like too much of a hack.
>>>
>>>> * add multiple symbols ?
>>>> should we have
>>>> (for <opname> in operator-list1, <opname2> in operator-list2
>>>>   (match_and_simplify
>>>>      ...))
>>>> or have nested for ?
>>>> (for <opname> in operator-list1
>>>>    (for <opname2> in operator-list2
>>>>        (match_and_simplify
>>>>           ....)))
>>>
>>> It depends on the desired semantics.  It's a lot clearer with
>>> single opname for only (but then we eventually need nested for).
>>>
>>>> * we don't detect functions with wrong arguments
>>>> for example, we dont give error on:
>>>> (built_in_sqrt @0 @1)
>>>> I guess that's because we don't have an easy way to figure out
>>>> number of arguments a function expects ?
>>>> (is there a built-in equivalent of tree_code_length[] ?)
>>>
>>> Yeah, the function stuff is still very simplistic and no, there isn't
>>> any easy way of extracting the number of expected arguments
>>> from builtins.def (it's encoded in the type).  The easiest way
>>> would be to change builtins.def to add a number-of-args argument ...
>>>
>>> Let's just defer that issue.
>>>
>>>
>>> As for the patch, we shouldn't do the cartesian_product - that's
>>> hardly ever what the user intends and it means there isn't any
>>> way of repeating the same pattern for multiple operators.
>>>
>>> A common use for 'for' would be (for OP in ne eq (...)) as most
>>> equality foldings are valid for ne and eq.  Another is
>>> for the various division kinds we have - trunc_div ceil_div floor_div
>>> and round_div (same for mod):
>>>
>>> (for op in trunc_div ceil_div floor_div round_div
>>>   (match_and_simplify
>>>      (op @0 integer_onep)
>>>      @0))
>>>
>>> (good example for match.pd to exercise the code)
>>>
>>> Can you fix the cartesian product thing (it should only simplify the
>>> patch)?
>> This version of the patch, removes cartesian_product, and reuses
>> id_base::id for user-defined symbols.
>>
>> eg:
>> (for op in plus minus
>>   (match_and_simplify
>>     (op (op @0 @1) @2)
>>     (op @0 @0)))
>>
>> generates following patterns:
>> (plus (plus @0 @1) @2) -> (plus @0 @0)
>> (minus (minus @0 @1) @2) -> (minus @0 @0)
>> Is this correct ?
>
> Yes.
>
>> I added the (for op in trunc_div floor_div ceil_div round_div ..)
>> pattern in match.pd
>> This works for trunc_div, I am not sure how to generate
>> floor_div/ceil_div/round_div
>> from C test-case.
>
> Yeah, I think all of them may only appear with variable-length array
> lowering.  No need to add testcases for them.
>
>> I added the following rotate pattern in match.pd:
>> /* (x << CNT1) OP (x >> CNT2) -> x r<< CNT1 OP being +, |, ^ */
>> (for op in plus bit_ior bit_xor
>> (match_and_simplify
>>   (op:c (lshift @0 INTEGER_CST_P@1) (rshift @0 INTEGER_CST_P@2))
>>   if (tree_fits_uhwi_p (@1) && tree_fits_uhwi_p (@2)
>>       && tree_to_uhwi (@1) + tree_to_uhwi (@2) == TYPE_PRECISION (type))
>>   (lrotate @0 @1)))
>>
>> Is this correct ?
>
> it doesn't work for signed x (rshift is arithmetic shift).  So you miss
> a TYPE_UNSIGNED (TREE_TYPE (@0)) check.  fold-const.c also
> verifies that mode and type precision match (and makes sure to
> use the vector/complex type element precision as shifts on
> vectors shift their elements).  See around fold-const.c:10562.
Thanks.
>
> I have fixed that, restricting it to integer types for now and committed
> the patch.
There appears to be some problem with the commit.
The Changelog.mas contains the correct details (for3.patch), but I think,
you committed for2.patch, the one i submitted earlier.
For instance Changelog.mas contains
* genmatch.c (parse_pattern): New function, while parse_pattern is not
defined in genmatch.c
Could you please revert and apply for3.patch ? I have attached it.

* genmatch.c (id_base::id_kind): Add new enum value USER_DEFINED.
  (e_operation::e_operation): New default argument add_new_id. Adjust
to add id in hash table.
  (simplify): Remove matchers member. New member match.
  (print_matches): Adjust to changes in simplify.
  (decision_tree::insert): Likewise.
  (lower_commutative): New function.
  (check_operator): Likewise.
  (replace_id): Likewise.
  (eat_ident): Likewise.
  (parse_for): Likewise.
  (parse_pattern): Likewise.
  (check_no_user_id): Likewise.
  (check_no_user_id): Likewise, overloaded to take simplify as first argument.
  (parse_expr): Call check_operator.
  (main): call parse_pattern, lower_commutative, check_no_user_id.

* match.pd: Adjust few constant folding patterns to use for.

[gcc/testsuite/gcc.dg/tree-ssa]
* match-rotate.c: New test-case.

Thanks and Regards,
Prathamesh
>
> Thanks,
> Richard.
>
>> * genmatch.c (id_base::id_kind): New enum value USER_DEFINED.
>>     (e_operation::e_operation): Add default argument add_new_id.
>>     (simplify): Remove member matchers.
>>     (simplify): New member match.
>>     (print_matches): Adjust to changes in simplify.
>>     (decision_tree::insert): Likewise.
>>     (parse_match_and_simplify): Likewise.
>>     (lower_commutative): New function.
>>     (check_operator): Likewise.
>>     (replace_id): Likewise.
>>     (eat_ident): Likewise.
>>     (parse_for): Likewise.
>>     (parse_expr): Call check_operator.
>>     (main): Call parse_for, lower_commutative.
>>
>> * match.pd: Add pattern for div, and rotate pattern.
>>
>> [gcc/testsuite/gcc.dg/tree-ssa]
>> * match-rotate.c: New test-case.
>>
>> Thanks and Regards,
>> Prathamesh
>>
>>>
>>> Thanks,
>>> Richard.
>>>
>>>> * genmatch.c (e_operation::e_operation): New constructor.
>>>> (e_operation::user_id): New member.
>>>> (e_operation::get_op): New member function.
>>>> (simplify::matchers): Remove.
>>>> (simplify::match): New member.
>>>> (lower_commutative): New function.
>>>> (check_operator): Likewise.
>>>> (replace_user_id): Likewise.
>>>> (decision_tree::insert): Adjust to changes in simplify.
>>>> (eat_ident): New function.
>>>> (parse_expr): Call to check_operator.
>>>> (parse_for): New function.
>>>> (main): Add calls to parse_for, lower_commutative.
>>>>
>>>> Thanks and Regards,
>>>> Prathamesh
>>>

Index: gcc/genmatch.c
===================================================================
--- gcc/genmatch.c	(revision 212366)
+++ gcc/genmatch.c	(working copy)
@@ -116,7 +116,7 @@ END_BUILTINS
 
 struct id_base : typed_free_remove<id_base>
 {
-  enum id_kind { CODE, FN } kind;
+  enum id_kind { CODE, FN, USER_DEFINED } kind;
 
   id_base (id_kind, const char *);
 
@@ -135,6 +135,7 @@ id_base::hash (const value_type *op)
 {
   return op->hashval;
 }
+
 inline int
 id_base::equal (const value_type *op1,
 			const compare_type *op2)
@@ -169,6 +170,7 @@ struct fn_id : public id_base
   enum built_in_function fn;
 };
 
+
 static void
 add_operator (enum tree_code code, const char *id,
 	      const char *tcc, unsigned nargs)
@@ -217,7 +219,7 @@ struct predicate : public operand
 };
 
 struct e_operation {
-  e_operation (const char *id, bool is_commutative_ = false);
+  e_operation (const char *id, bool is_commutative_ = false, bool add_new_id = true);
   id_base *op;
   bool is_commutative;
 };
@@ -254,11 +256,10 @@ struct capture : public operand
   virtual void gen_transform (FILE *f, const char *, bool);
 };
 
-
-e_operation::e_operation (const char *id, bool is_commutative_)
+e_operation::e_operation (const char *id, bool is_commutative_, bool add_new_id)
 {
-  id_base tem (id_base::CODE, id);
   is_commutative = is_commutative_;
+  id_base tem (id_base::CODE, id);
 
   op = operators.find_with_hash (&tem, tem.hashval);
   if (op)
@@ -287,19 +288,23 @@ e_operation::e_operation (const char *id
       return;
     }
 
-  fatal ("expected operator, got %s", id);
+  if (add_new_id == false)
+    fatal ("%s is not an operator/built-in function", id);
+
+  op = new id_base (id_base::USER_DEFINED, id);
+  operators.find_slot_with_hash (op, op->hashval, INSERT);
 }
 
 struct simplify {
   simplify (const char *name_,
-	    vec<operand *> matchers_, source_location match_location_,
+	    operand *match_, source_location match_location_,
 	    struct operand *ifexpr_, source_location ifexpr_location_,
 	    struct operand *result_, source_location result_location_)
-      : name (name_), matchers (matchers_), match_location (match_location_),
+      : name (name_), match (match_), match_location (match_location_),
       ifexpr (ifexpr_), ifexpr_location (ifexpr_location_),
       result (result_), result_location (result_location_) {}
   const char *name;
-  vec<operand *> matchers;  // vector to hold commutative expressions
+  operand *match; 
   source_location match_location;
   struct operand *ifexpr;
   source_location ifexpr_location;
@@ -452,19 +457,9 @@ print_operand (operand *o, FILE *f = std
 void
 print_matches (struct simplify *s, FILE *f = stderr)
 {
-  if (s->matchers.length () == 1)
-    return;
-
   fprintf (f, "for expression: ");
-  print_operand (s->matchers[0], f);  // s->matchers[0] is equivalent to original expression
+  print_operand (s->match, f); 
   putc ('\n', f);
-
-  fprintf (f, "commutative expressions:\n");
-  for (unsigned i = 0; i < s->matchers.length (); ++i)
-    {
-      print_operand (s->matchers[i], f);
-      putc ('\n', f);
-    }
 }
 
 void
@@ -552,6 +547,104 @@ commutate (operand *op)
   return ret;
 }
 
+void
+lower_commutative (simplify *s, vec<simplify *>& simplifiers)
+{
+  vec<operand *> matchers = commutate (s->match);
+  for (unsigned i = 0; i < matchers.length (); ++i)
+    {
+      simplify *ns = new simplify (s->name, matchers[i], s->match_location,
+				   s->ifexpr, s->ifexpr_location,
+				   s->result, s->result_location);
+      simplifiers.safe_push (ns);
+    }
+}
+
+void
+check_operator (id_base *op, unsigned n_ops, const cpp_token *token = 0)
+{
+  if (!op)
+    return;
+
+  if (op->kind != id_base::CODE)
+    return;
+
+  operator_id *opr = static_cast<operator_id *> (op);
+  if (opr->get_required_nargs () == n_ops)
+    return;
+
+  if (token)
+    fatal_at (token, "%s expects %u operands, got %u operands", opr->id, opr->get_required_nargs (), n_ops);
+  else
+    fatal ("%s expects %u operands, got %u operands", opr->id, opr->get_required_nargs (), n_ops);
+}
+        
+operand *
+replace_id (operand *o, const char *user_id, const char *oper)
+{
+  if (o->type == operand::OP_CAPTURE)
+    {
+      capture *c = static_cast<capture *> (o);
+      if (!c->what)
+	return c;
+      capture *nc = new capture (c->where, replace_id (c->what, user_id, oper));
+      return nc;
+    }
+
+  if (o->type != operand::OP_EXPR)
+    return o;
+
+  expr *e = static_cast<expr *> (o);
+  expr *ne;
+
+  if (e->operation->op->kind == id_base::USER_DEFINED && strcmp (e->operation->op->id, user_id) == 0)
+    {
+      struct e_operation *operation = new e_operation (oper, e->operation->is_commutative, false);
+      check_operator (operation->op, e->ops.length ());
+      ne = new expr (operation);
+    }
+  else
+    ne = new expr (e->operation);
+
+  for (unsigned i = 0; i < e->ops.length (); ++i)
+    ne->append_op (replace_id (e->ops[i], user_id, oper));
+
+  return ne;
+}
+  
+void
+check_no_user_id (operand *o)
+{
+  if (o->type == operand::OP_CAPTURE)
+    {
+      capture *c = static_cast<capture *> (o);
+      if (c->what && c->what->type == operand::OP_EXPR)
+	{
+	  o = c->what;
+	  goto check_expr; 
+	}
+      return; 
+    }
+
+  if (o->type != operand::OP_EXPR)
+    return;
+
+check_expr:
+  expr *e = static_cast<expr *> (o);
+  if (e->operation->op->kind == id_base::USER_DEFINED)
+    fatal ("%s is not defined in for", e->operation->op->id);
+
+  for (unsigned i = 0; i < e->ops.length (); ++i)
+    check_no_user_id (e->ops[i]);
+}
+
+void
+check_no_user_id (simplify *s)
+{
+  check_no_user_id (s->match);
+  check_no_user_id (s->result);
+}
+
 /* Code gen off the AST.  */
 
 void
@@ -828,17 +921,14 @@ decision_tree::insert (struct simplify *
 {
   dt_operand *indexes[dt_simplify::capture_max];
 
-  for (unsigned i = 0; i < s->matchers.length (); ++i)
-    {
-      if (s->matchers[i]->type != operand::OP_EXPR)
-	continue;
+  if (s->match->type != operand::OP_EXPR)
+    return; 
 
-      for (unsigned j = 0; j < dt_simplify::capture_max; ++j)
-	indexes[j] = 0; 
+  for (unsigned j = 0; j < dt_simplify::capture_max; ++j)
+    indexes[j] = 0; 
 
-      dt_node *p = decision_tree::insert_operand (root, s->matchers[i], indexes);
-      p->append_simplify (s, pattern_no, indexes);
-    }            
+  dt_node *p = decision_tree::insert_operand (root, s->match, indexes);
+  p->append_simplify (s, pattern_no, indexes);
 }
 
 void
@@ -1707,6 +1797,16 @@ get_ident (cpp_reader *r)
   return (const char *)CPP_HASHNODE (token->val.node.node)->ident.str;
 }
 
+static void
+eat_ident (cpp_reader *r, const char *s)
+{
+  const cpp_token *token = expect (r, CPP_NAME);
+  const char *t = (const char *) CPP_HASHNODE (token->val.node.node)->ident.str;
+
+  if (strcmp (s, t)) 
+    fatal_at (token, "expected %s got %s\n", s, t);
+}
+
 /* Read the next token from R and assert it is of type CPP_NUMBER and
    return its value.  */
 
@@ -1735,6 +1835,7 @@ parse_capture (cpp_reader *r, operand *o
   return new capture (get_number (r), op);
 }
 
+
 /* Parse
      expr = (operation[capture] op...)  */
 static struct operand *
@@ -1774,13 +1875,7 @@ parse_expr (cpp_reader *r)
       const cpp_token *token = peek (r);
       if (token->type == CPP_CLOSE_PAREN)
 	{
-	  if (e->operation->op->kind == id_base::CODE)
-	    {
-	      operator_id *opr = static_cast <operator_id *> (e->operation->op);
-	      if (e->ops.length () != opr->get_required_nargs ())
-		fatal_at (token, "got %d operands instead of the required %d",
-			  e->ops.length (), opr->get_required_nargs ());
-	    }
+	  check_operator (e->operation->op, e->ops.length (), token); 
 	  if (is_commutative)
 	    {
 	      if (e->ops.length () == 2)
@@ -1877,6 +1972,7 @@ parse_op (cpp_reader *r)
   return op;
 }
 
+
 /* Parse
      (define_match_and_simplify "<ident>"
         <op> <op>)  */
@@ -1912,10 +2008,44 @@ parse_match_and_simplify (cpp_reader *r,
       ifexpr = parse_c_expr (r, CPP_OPEN_PAREN);
     }
   token = peek (r);
-  return new simplify (id, commutate (match), match_location,
+  return new simplify (id, match, match_location,
 		       ifexpr, ifexpr_location, parse_op (r), token->src_loc);
 }
 
+void
+parse_for (cpp_reader *r, source_location match_location, vec<simplify *>& simplifiers) 
+{
+  const char *user_id = get_ident (r);
+  eat_ident (r, "in");
+  void parse_pattern (cpp_reader *, vec<simplify *>&);
+
+  vec<const char *> opers = vNULL;
+
+  while (1)
+    {
+      const cpp_token *token = peek (r);
+      if (token->type != CPP_NAME)
+	break;
+      opers.safe_push (get_ident (r));
+    }
+
+  vec<simplify *> for_simplifiers = vNULL;
+  parse_pattern (r, for_simplifiers);
+
+  for (unsigned i = 0; i < opers.length (); ++i)
+    {
+      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]);
+	  simplify *ns = new simplify (s->name, match_op, s->match_location,
+				       s->ifexpr, s->ifexpr_location,
+				       result_op, s->result_location);
+	  simplifiers.safe_push (ns);
+	}
+    }
+}
 
 static size_t
 round_alloc_size (size_t s)
@@ -1923,6 +2053,23 @@ round_alloc_size (size_t s)
   return s;
 }
 
+void
+parse_pattern (cpp_reader *r, vec<simplify *>& simplifiers)
+{
+  /* All clauses start with '('.  */
+  eat_token (r, CPP_OPEN_PAREN);
+  const cpp_token *token = peek (r);
+  const char *id = get_ident (r);
+  if (strcmp (id, "match_and_simplify") == 0)
+    simplifiers.safe_push (parse_match_and_simplify (r, token->src_loc));
+  else if (strcmp (id, "for") == 0)
+    parse_for (r, token->src_loc, simplifiers); 
+  else
+    fatal_at (token, "expected 'match_and_simplify' or 'for'");
+
+  eat_token (r, CPP_CLOSE_PAREN);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -1974,42 +2121,35 @@ main(int argc, char **argv)
 
   vec<simplify *> simplifiers = vNULL;
 
-  do
-    {
-      token = peek (r);
-      if (token->type == CPP_EOF)
-	break;
-
-      /* All clauses start with '('.  */
-      eat_token (r, CPP_OPEN_PAREN);
-
-      const char *id = get_ident (r);
-      if (strcmp (id, "match_and_simplify") == 0)
-	simplifiers.safe_push (parse_match_and_simplify (r, token->src_loc));
-      else
-	fatal_at (token, "expected 'match_and_simplify'");
+  while (peek (r)->type != CPP_EOF)
+    parse_pattern (r, simplifiers);
 
-      eat_token (r, CPP_CLOSE_PAREN);
+  for (unsigned i = 0; i < simplifiers.length (); ++i)
+    {
+      fprintf (stderr, "pattern = %u\n", i);
+      check_no_user_id (simplifiers[i]);
     }
-  while (1);
-
+  vec<simplify *> out_simplifiers = vNULL;
   for (unsigned i = 0; i < simplifiers.length (); ++i)
-    print_matches (simplifiers[i]);
+    lower_commutative (simplifiers[i], out_simplifiers);
+
+  for (unsigned i = 0; i < out_simplifiers.length (); ++i)
+    print_matches (out_simplifiers[i]);
 
   decision_tree dt;
-  for (unsigned i = 0; i < simplifiers.length (); ++i)
-    dt.insert (simplifiers[i], i);
+  for (unsigned i = 0; i < out_simplifiers.length (); ++i)
+    dt.insert (out_simplifiers[i], i);
 
   dt.print (stderr);
  
   if (gimple)
     {
-      write_header (stdout, simplifiers, "gimple-match-head.c");
+      write_header (stdout, out_simplifiers, "gimple-match-head.c");
       dt.gen_gimple (stdout);
     }
   else
     {
-      write_header (stdout, simplifiers, "generic-match-head.c");
+      write_header (stdout, out_simplifiers, "generic-match-head.c");
       dt.gen_generic (stdout);
     }
 
Index: gcc/match.pd
===================================================================
--- gcc/match.pd	(revision 212366)
+++ gcc/match.pd	(working copy)
@@ -22,29 +22,26 @@ along with GCC; see the file COPYING3.
 <http://www.gnu.org/licenses/>.  */
 
 /* Simple constant foldings to substitute gimple_fold_stmt_to_constant_2.  */
-(match_and_simplify
-  (plus @0 integer_zerop)
-  @0)
-(match_and_simplify
-  (pointer_plus @0 integer_zerop)
-  @0)
-(match_and_simplify
-  (minus @0 integer_zerop)
-  @0)
+(for op in plus pointer_plus minus bit_ior bit_xor
+  (match_and_simplify
+    (op @0 integer_zerop)
+    @0))
+
 (match_and_simplify
   (minus @0 @0)
   { build_zero_cst (type); })
+
 (match_and_simplify
   (mult @0 integer_zerop@1)
   @1)
-(match_and_simplify
-  (mult @0 integer_onep)
-  @0)
+
 /* Make sure to preserve divisions by zero.  This is the reason why
    we don't simplify x / x to 1 or 0 / x to 0.  */
-(match_and_simplify
-  (trunc_div @0 integer_onep)
-  @0)
+(for op in mult trunc_div ceil_div floor_div round_div
+  (match_and_simplify
+    (op @0 integer_onep)
+    @0))
+
 (match_and_simplify
   (trunc_mod @0 integer_onep)
   { build_zero_cst (type); })
@@ -55,9 +52,6 @@ along with GCC; see the file COPYING3.
   if (!integer_zerop (@1))
   @0)
 (match_and_simplify
-  (bit_ior @0 integer_zerop)
-  @0)
-(match_and_simplify
   (bit_ior @0 integer_all_onesp@1)
   @1)
 (match_and_simplify
@@ -67,9 +61,6 @@ along with GCC; see the file COPYING3.
   (bit_and @0 integer_zerop@1)
   @1)
 (match_and_simplify
-  (bit_xor @0 integer_zerop)
-  @0)
-(match_and_simplify
   (bit_xor @0 @0)
   { build_zero_cst (type); })
 /* tree-ssa/ifc-pr44710.c requires a < b ? c : d to fold to 1.
@@ -358,6 +349,15 @@ along with GCC; see the file COPYING3.
   if (INTEGRAL_TYPE_P (TREE_TYPE (@0)))
   { build_int_cst (type, 0); })
 
+/* (x << CNT1) OP (x >> CNT2) -> x r<< CNT1 OP being +, |, ^ */
+(for op in plus bit_ior bit_xor 
+(match_and_simplify
+  (op:c (lshift @0 INTEGER_CST_P@1) (rshift @0 INTEGER_CST_P@2))
+  if (tree_fits_uhwi_p (@1) && tree_fits_uhwi_p (@2)
+      && tree_to_uhwi (@1) + tree_to_uhwi (@2) == TYPE_PRECISION (type))
+  (lrotate @0 @1)))
+
+
 /* ????s
 
    We cannot reasonably match vector CONSTRUCTORs or vector constants
Index: gcc/testsuite/gcc.dg/tree-ssa/match-rotate.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/match-rotate.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/match-rotate.c	(working copy)
@@ -0,0 +1,44 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-forwprop1-details" }  */
+
+unsigned char
+rotate_1 (unsigned char x)
+{
+  unsigned char t1 = x << 5;
+  unsigned char t2 = x >> 3;
+  unsigned char rotate_1_val = t1 + t2;
+  return rotate_1_val;
+}
+/* { dg-final { scan-tree-dump "gimple_match_and_simplified to rotate_1_val_\\d\+ = x_\\d\+\\(D\\) r<< 5" "forwprop1" } } */
+
+unsigned char
+rotate_2 (unsigned char x)
+{
+  unsigned char t1 = x << 4;
+  unsigned char t2 = x >> 3;
+  unsigned char rotate_2_val = t1 + t2;
+  return rotate_2_val;
+}
+/* { dg-final { scan-tree-dump-not "gimple_match_and_simplified to rotate_2_val_\\d\+ = x_\\d\+\\(D\\) r<< 5" "forwprop1" } } */
+
+unsigned char
+rotate_3 (unsigned char x)
+{
+  unsigned char t1 = x << 5;
+  unsigned char t2 = x >> 3;
+  unsigned char rotate_3_val = t1 | t2;
+  return rotate_3_val;
+}
+/* { dg-final { scan-tree-dump "gimple_match_and_simplified to rotate_3_val_\\d\+ = x_\\d\+\\(D\\) r<< 5" "forwprop1" } } */
+
+unsigned char
+rotate_4 (unsigned char x)
+{
+  unsigned char t1 = x << 5;
+  unsigned char t2 = x >> 3;
+  unsigned char rotate_4_val = t1 ^ t2;
+  return rotate_4_val;
+}
+/* { dg-final { scan-tree-dump "gimple_match_and_simplified to rotate_4_val_\\d\+ = x_\\d\+\\(D\\) r<< 5" "forwprop1" } } */
+
+/* { dg-final { cleanup-tree-dump "forwprop1" } } */