gimple-walk.c #include TLC

Hi there,

I noticed that gimple-walk.c has a creative list of #includes.
Furthermore, in walk_gimple_asm parse_{in,out}put_constraint was called
even if neither allows_mem, allows_reg nor is_inout were used -- i.e. if
wi is NULL -- and the return value of the constraint parsing was not
taken into account which looks wrong or at least odd. Note that several
other spots in the tree do ignore the parse_{in,out}put_constraint return
values and should be adjusted too AFAIU. Otherwise we might attempt
(and use!) to extract information from otherwise illegal constraints,
it seems?

Bootstrapped and regtested on x86_64-unknown-linux with no regressions.
Ok for trunk?

gcc/ChangeLog:

	* gimple-walk.c: Prune duplicate or unneeded includes.
	(walk_gimple_asm): Only call parse_input_constraint or
	parse_output_constraint if their findings are used.
	Honour parse_input_constraint and parse_output_constraint
	result.

On Wed, Apr 29, 2015 at 10:01 AM, Bernhard Reutner-Fischer
<rep.dot.nop@gmail.com> wrote:
> Hi there,
>
> I noticed that gimple-walk.c has a creative list of #includes.
> Furthermore, in walk_gimple_asm parse_{in,out}put_constraint was called
> even if neither allows_mem, allows_reg nor is_inout were used -- i.e. if
> wi is NULL -- and the return value of the constraint parsing was not
> taken into account which looks wrong or at least odd. Note that several
> other spots in the tree do ignore the parse_{in,out}put_constraint return
> values and should be adjusted too AFAIU. Otherwise we might attempt
> (and use!) to extract information from otherwise illegal constraints,
> it seems?
>
> Bootstrapped and regtested on x86_64-unknown-linux with no regressions.
> Ok for trunk?

Ok.

Thanks,
Richard.

> gcc/ChangeLog:
>
>         * gimple-walk.c: Prune duplicate or unneeded includes.
>         (walk_gimple_asm): Only call parse_input_constraint or
>         parse_output_constraint if their findings are used.
>         Honour parse_input_constraint and parse_output_constraint
>         result.
>
> diff --git a/gcc/gimple-walk.c b/gcc/gimple-walk.c
> index 45ff859..53462b5 100644
> --- a/gcc/gimple-walk.c
> +++ b/gcc/gimple-walk.c
> @@ -2,75 +2,69 @@
>
>     Copyright (C) 2007-2015 Free Software Foundation, Inc.
>     Contributed by Aldy Hernandez <aldyh@redhat.com>
>
>  This file is part of GCC.
>
>  GCC is free software; you can redistribute it and/or modify it under
>  the terms of the GNU General Public License as published by the Free
>  Software Foundation; either version 3, or (at your option) any later
>  version.
>
>  GCC is distributed in the hope that it will be useful, but WITHOUT ANY
>  WARRANTY; without even the implied warranty of MERCHANTABILITY or
>  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>  for more details.
>
>  You should have received a copy of the GNU General Public License
>  along with GCC; see the file COPYING3.  If not see
>  <http://www.gnu.org/licenses/>.  */
>
>  #include "config.h"
>  #include "system.h"
>  #include "coretypes.h"
>  #include "tm.h"
>  #include "hash-set.h"
> -#include "machmode.h"
>  #include "vec.h"
>  #include "double-int.h"
>  #include "input.h"
>  #include "alias.h"
>  #include "symtab.h"
> -#include "wide-int.h"
>  #include "inchash.h"
>  #include "tree.h"
> -#include "fold-const.h"
> -#include "stmt.h"
>  #include "predict.h"
>  #include "hard-reg-set.h"
> -#include "input.h"
>  #include "function.h"
> -#include "basic-block.h"
> -#include "tree-ssa-alias.h"
> -#include "internal-fn.h"
>  #include "gimple-expr.h"
>  #include "is-a.h"
> +#include "tree-ssa-alias.h"
> +#include "basic-block.h"
> +#include "fold-const.h"
>  #include "gimple.h"
>  #include "gimple-iterator.h"
>  #include "gimple-walk.h"
> -#include "gimple-walk.h"
> -#include "demangle.h"
> +#include "stmt.h"
>
>  /* Walk all the statements in the sequence *PSEQ calling walk_gimple_stmt
>     on each one.  WI is as in walk_gimple_stmt.
>
>     If walk_gimple_stmt returns non-NULL, the walk is stopped, and the
>     value is stored in WI->CALLBACK_RESULT.  Also, the statement that
>     produced the value is returned if this statement has not been
>     removed by a callback (wi->removed_stmt).  If the statement has
>     been removed, NULL is returned.
>
>     Otherwise, all the statements are walked and NULL returned.  */
>
>  gimple
>  walk_gimple_seq_mod (gimple_seq *pseq, walk_stmt_fn callback_stmt,
>                      walk_tree_fn callback_op, struct walk_stmt_info *wi)
>  {
>    gimple_stmt_iterator gsi;
>
>    for (gsi = gsi_start (*pseq); !gsi_end_p (gsi); )
>      {
>        tree ret = walk_gimple_stmt (&gsi, callback_stmt, callback_op, wi);
>        if (ret)
>         {
>           /* If CALLBACK_STMT or CALLBACK_OP return a value, WI must exist
>              to hold it.  */
> @@ -107,71 +101,76 @@ walk_gimple_seq (gimple_seq seq, walk_stmt_fn callback_stmt,
>
>  /* Helper function for walk_gimple_stmt.  Walk operands of a GIMPLE_ASM.  */
>
>  static tree
>  walk_gimple_asm (gasm *stmt, walk_tree_fn callback_op,
>                  struct walk_stmt_info *wi)
>  {
>    tree ret, op;
>    unsigned noutputs;
>    const char **oconstraints;
>    unsigned i, n;
>    const char *constraint;
>    bool allows_mem, allows_reg, is_inout;
>
>    noutputs = gimple_asm_noutputs (stmt);
>    oconstraints = (const char **) alloca ((noutputs) * sizeof (const char *));
>
>    if (wi)
>      wi->is_lhs = true;
>
>    for (i = 0; i < noutputs; i++)
>      {
>        op = gimple_asm_output_op (stmt, i);
>        constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (op)));
>        oconstraints[i] = constraint;
> -      parse_output_constraint (&constraint, i, 0, 0, &allows_mem, &allows_reg,
> -                              &is_inout);
>        if (wi)
> -       wi->val_only = (allows_reg || !allows_mem);
> +       {
> +         if (parse_output_constraint (&constraint, i, 0, 0, &allows_mem,
> +                                      &allows_reg, &is_inout))
> +           wi->val_only = (allows_reg || !allows_mem);
> +       }
>        ret = walk_tree (&TREE_VALUE (op), callback_op, wi, NULL);
>        if (ret)
>         return ret;
>      }
>
>    n = gimple_asm_ninputs (stmt);
>    for (i = 0; i < n; i++)
>      {
>        op = gimple_asm_input_op (stmt, i);
>        constraint = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (op)));
> -      parse_input_constraint (&constraint, 0, 0, noutputs, 0,
> -                             oconstraints, &allows_mem, &allows_reg);
> +
>        if (wi)
>         {
> -         wi->val_only = (allows_reg || !allows_mem);
> -          /* Although input "m" is not really a LHS, we need a lvalue.  */
> -         wi->is_lhs = !wi->val_only;
> +         if (parse_input_constraint (&constraint, 0, 0, noutputs, 0,
> +                                     oconstraints, &allows_mem, &allows_reg))
> +           {
> +             wi->val_only = (allows_reg || !allows_mem);
> +             /* Although input "m" is not really a LHS, we need a lvalue.  */
> +             wi->is_lhs = !wi->val_only;
> +           }
>         }
>        ret = walk_tree (&TREE_VALUE (op), callback_op, wi, NULL);
>        if (ret)
>         return ret;
>      }
>
>    if (wi)
>      {
>        wi->is_lhs = false;
>        wi->val_only = true;
>      }
>
>    n = gimple_asm_nlabels (stmt);
>    for (i = 0; i < n; i++)
>      {
>        op = gimple_asm_label_op (stmt, i);
>        ret = walk_tree (&TREE_VALUE (op), callback_op, wi, NULL);
>        if (ret)
>         return ret;
>      }
>
>    return NULL_TREE;
>  }
>
>

On 29 April 2015 at 11:00, Richard Biener <richard.guenther@gmail.com> wrote:
> On Wed, Apr 29, 2015 at 10:01 AM, Bernhard Reutner-Fischer
> <rep.dot.nop@gmail.com> wrote:
>> Hi there,
>>
>> I noticed that gimple-walk.c has a creative list of #includes.
>> Furthermore, in walk_gimple_asm parse_{in,out}put_constraint was called
>> even if neither allows_mem, allows_reg nor is_inout were used -- i.e. if
>> wi is NULL -- and the return value of the constraint parsing was not
>> taken into account which looks wrong or at least odd. Note that several
>> other spots in the tree do ignore the parse_{in,out}put_constraint return
>> values and should be adjusted too AFAIU. Otherwise we might attempt
>> (and use!) to extract information from otherwise illegal constraints,
>> it seems?
>>
>> Bootstrapped and regtested on x86_64-unknown-linux with no regressions.
>> Ok for trunk?
>
> Ok.

r222569.
Thanks!