Re: [GSoC][match-and-simplify] factor gimple expressions and builtin functions

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Jun 26, 2014 at 11:43 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Thu, Jun 26, 2014 at 10:11 AM, Prathamesh Kulkarni
> <bilbotheelffriend@gmail.com> wrote:
>> This patch factors expression checking for GIMPLE.
>>
>> Generates code as:
>> if (TREE_CODE (opname) == SSA_NAME)
>> {
>>   gimple def_stmt = SSA_NAME_DEF_STMT (opname);
>>   if (is_gimple_assign (def_stmt))
>>   {
>>     if (gimple_assign_rhs_code (def_stmt) == <expr-code1>)
>>       {
>>          ....
>>       }
>>     if (gimple_assign_rhs_code (def_stmt) == <expr-code2>)
>>       {
>>          ....
>>       }
>>    }
>> }
>>
>> We cannot use switch-case, for convert/nop since
>> we use CONVERT_EXPR_CODE_P, so i used if stmt.
>
> Actually we can by using
>
>    case NOP_EXPR:
>    case CONVERT_EXPR:
>
> for it.  Note that currently you still do
>
>   if (is_gimple_assign (def_stmt))
>     {
>        if (gimple_assign_rhs_code (def_stmt) == PLUS_EXPR)
>         {
>         }
>       if (gimple_assign_rhs_code (def_stmt) == MINUS_EXPR)
>         {
>         }
>
> thus not use else if ().  Of course in the end we want
>
>   switch (gimple_assign_rhs_code (def_stmt))
>    {
>     case PLUS_EXPR;
>   ...
>     case MINUS_EXPR:
>   ...
>
>> Unfortunately, back-tracking is still done. I shall look into that.
>
> Note that we need to backtrack to the next parent with a 'true'
> kid (and from there to its next parent with a 'true' kid).  Possibly
> with using a switch-case or proper if-else-if there isn't anything
> special to do as the backtracking would work naturally then.
>
> So I suggest to concentrate on getting the code to use
>
>   if (TREE_CODE (op0) == SSA_NAME)
>     {
>        gimple def_stmt = SSA_NAME_DEF_STMT (op0);
>        if (is_gimple_assign  (def_stmt))
>          {
>            switch (gimple_assign_rhs_code ())
>             {
>             case PLUS_EXPR:
> ...
>          }
>        else if (gimple_call_builtin_p (def_stmt, BUILT_IN_NORMAL))
>          {
>            switch (DECL_FUNCTION_CODE (gimple_call_fndecl (def_stmt)))
>             {
>             case BUILT_IN_ABS:
> ...
>          }
>      }
>   else if (TREE_CODE (op0) == REALPART_EXPR)
> ... other GENERIC cases
>
>   <true> match (not in else {})
>
> that would backtrack through the <true> cases by means of them
> not short-circuited by an else but everything else short-circuited
> by proper use of switch () and if - else-if code.

Actually it is quite a bit more complicated.  While OP_EXPR naturally
short-circuit a failed DT_MATCH doesn't mean we haven't to check
other DT_OPERANDs - likewise we have to test all OP_PREDICATE
until we find a matching one.

So we should emit OP_EXPR checks first, short-circuiting each other
and then following with DT_MATCH and OP_PREDICATE ones,
but not short-circuiting.

I have tried to hack that into GENERIC code-gen but the current
code-gen structure is somewhat awkward ... (see patch in separate
thread).

I see you get around most of the ugliness by using grok_kids,
but with a sorted kids array this would come for free I think
(sorted after desired code-gen order).

Feel free to comment the GENERIC code-gen (just emit empty
functions) when you want to refactor things.

I will be on vacation for the next week.

Richard.

> Thanks,
> Richard.
>
>> * genmatch.c (dt_node::get_expr_code): New member function.
>>      (dt_node::is_gimple_expr): Likewise.
>>      (dt_node::is_gimple_fn): Likewise.
>>      (dt_operand::kids_type): New struct.
>>      (dt_operand::gen_gimple_expr): Remove.
>>      (dt_operand::gen_gimple_expr_expr): Remove 2nd argument and
>> change return        type to unsigned. Adjust code-gen for
>> expressions.
>>      (dt_operand::gen_gimple_expr_fn): Remove 2nd argument and change
>> return type to unsigned.
>>      (dt_operand::grok_kids): New member function.
>>      (dt_operand::gen_gimple_kids): Likewise.
>>      (dt_operand::gen_gimple): Adjust code-gen for gimple expressions.
>> Call dt_operand::gen_gimple_kids.
>>      (decision_tree::gen_gimple): Call dt_operand::gen_gimple_kids.
>>
>> Thanks and Regards,
>> Prathamesh