gimple-walk.c #include TLC
diff mbox

Message ID 1430294510-18361-2-git-send-email-rep.dot.nop@gmail.com
State New
Headers show

Commit Message

Bernhard Reutner-Fischer April 29, 2015, 8:01 a.m. UTC
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.

Comments

Richard Biener April 29, 2015, 9 a.m. UTC | #1
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;
>  }
>
>
Bernhard Reutner-Fischer April 29, 2015, 10:46 a.m. UTC | #2
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!

Patch
diff mbox

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;
 }