This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] gimple-walk.c #include TLC
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Bernhard Reutner-Fischer <rep dot dot dot nop at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 29 Apr 2015 11:00:01 +0200
- Subject: Re: [PATCH] gimple-walk.c #include TLC
- Authentication-results: sourceware.org; auth=none
- References: <1430294510-18361-1-git-send-email-rep dot dot dot nop at gmail dot com> <1430294510-18361-2-git-send-email-rep dot dot dot nop at gmail dot com>
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;
> }
>
>