On Wed, Apr 29, 2015 at 10:01 AM, Bernhard Reutner-Fischer <rep.dot....@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 <al...@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; > } > >