On Sat, Aug 16, 2014 at 3:46 PM, Prathamesh Kulkarni
<bilbotheelffri...@gmail.com> wrote:
> On Mon, Aug 11, 2014 at 4:58 PM, Richard Biener
> <richard.guent...@gmail.com> wrote:
>> On Sun, Aug 10, 2014 at 11:17 PM, Prathamesh Kulkarni
>> <bilbotheelffri...@gmail.com> wrote:
>>> On Mon, Aug 4, 2014 at 2:13 PM, Richard Biener
>>> <richard.guent...@gmail.com> wrote:
>>>> On Sun, Aug 3, 2014 at 6:58 PM, Prathamesh Kulkarni
>>>> <bilbotheelffri...@gmail.com> wrote:
>>>>> On Tue, Jul 29, 2014 at 4:29 PM, Richard Biener
>>>>> <richard.guent...@gmail.com> wrote:
>>>>>> On Mon, Jul 28, 2014 at 10:02 PM, Prathamesh Kulkarni
>>>>>> <bilbotheelffri...@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 <bilbotheelffri...@gmail.com>
Richard Biener <rguent...@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)))
