[tuples] start c_gimple_diagnostics_recursively conversion

[tuples] start c_gimple_diagnostics_recursively conversion

[Aldy Hernandez]

I'm moving the exit we have for -fgimple-only past genericizing into
c_gimple_diagnostics_recursively() so we can make it further into the
compilation process.  Consequently, I'm trying to do some tuple
conversion work beyond the gimplifier.

This opened a few cans of wormlets, that I've fixed throughout.

First, we weren't handling calls through a pointer.  Fixed.

We also had try/catch arguments as tuples instead of sequences.  Fixed.

Finally, I've converted c_warn_unused_result() to work with our now
gimplified body.

No regressions.  Committing to trunk.

	* testsuite/gcc.dg/gimple/call2.c: New.
	* c-decl.c: Include gimple.h.
	(c_gimple_diagnostics_recursively): Call c_warn_unused_result with
	tuplified body.
	Exit if -fgimple-only.
	* gimplify.c (gimplify_call_expr): Call gimple_build_call_vec with
	the correct callee.
	* c-gimplify.c (c_genericize): Remove exit.
	* c-common.c: Include gimple.h.
	(c_warn_unused_result): Tuplify.
	* c-common.h (c_warn_unused_result): Protoize.
	* Makefile.in (c-decl.o): Depend on GIMPLE_H.
	(c-common.o): Same.
	* gimple.c (gimple_build_catch): Handler is a sequence.
	Adjust accordingly.
	(gimple_build_eh_filter): Failure is a sequence.
	Adjust accordingly.
	(walk_tuple_ops): case GIMPLE_CATCH: Walk handler as a sequence.
	case GIMPLE_EH_FILTER: Walkder failure as a sequence.
	* gimple.h (gimple_statement_catch): Make handler a sequence.
	(gimple_statement_eh_filter): Make failure a sequence.
	(gimple_build_catch): Make second argument a sequence.
	(gimple_build_eh_filter): Same.
	(gimple_catch_handler): Return a sequence.
	(gimple_catch_set_handler): Make second argument a sequence.
	(gimple_eh_filter_failure): Return a sequence.
	(gimple_eh_filter_set_failture): Make second argument a sequence.

Index: testsuite/gcc.dg/gimple/call2.c
===================================================================
--- testsuite/gcc.dg/gimple/call2.c	(revision 0)
+++ testsuite/gcc.dg/gimple/call2.c	(revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+
+int (*bar)();
+
+void foo(){
+  bar();
+}
+
+/* { dg-final { cleanup-tree-dump "gimple" } } */
Index: c-decl.c
===================================================================
--- c-decl.c	(revision 126798)
+++ c-decl.c	(working copy)
@@ -61,6 +61,7 @@ Software Foundation, 51 Franklin Street,
 #include "except.h"
 #include "langhooks-def.h"
 #include "pointer-set.h"
+#include "gimple.h"
 
 /* In grokdeclarator, distinguish syntactic contexts of declarators.  */
 enum decl_context
@@ -6682,9 +6683,13 @@ static void
 c_gimple_diagnostics_recursively (tree fndecl)
 {
   struct cgraph_node *cgn;
+  gimple_seq body = gimple_body (fndecl);
 
   /* Handle attribute((warn_unused_result)).  Relies on gimple input.  */
-  c_warn_unused_result (&DECL_SAVED_TREE (fndecl));
+  c_warn_unused_result (body);
+
+  if (flag_gimple_only)
+    exit (0);
 
   /* Notice when OpenMP structured block constraints are violated.  */
   if (flag_openmp)
Index: gimplify.c
===================================================================
--- gimplify.c	(revision 126803)
+++ gimplify.c	(working copy)
@@ -2180,7 +2180,7 @@ gimplify_call_expr (tree *expr_p, gimple
 
   /* Now add the GIMPLE call to PRE_P.  If WANT_VALUE is set, we need
      to create the appropriate temporary for the call's LHS.  */
-  call = gimple_build_call_vec (fndecl, args);
+  call = gimple_build_call_vec (CALL_EXPR_FN (*expr_p), args);
   gimple_add (pre_p, call);
   if (want_value)
     {
Index: c-gimplify.c
===================================================================
--- c-gimplify.c	(revision 126798)
+++ c-gimplify.c	(working copy)
@@ -107,9 +107,6 @@ c_genericize (tree fndecl)
 
   dump_function (TDI_generic, fndecl);
 
-  if (flag_gimple_only)
-    exit (0);
-
   /* Genericize all nested functions now.  We do things in this order so
      that items like VLA sizes are expanded properly in the context of
      the correct function.  */
Index: c-common.c
===================================================================
--- c-common.c	(revision 126798)
+++ c-common.c	(working copy)
@@ -49,6 +49,7 @@ Software Foundation, 51 Franklin Street,
 #include "real.h"
 #include "cgraph.h"
 #include "target-def.h"
+#include "gimple.h"
 
 cpp_reader *parse_in;		/* Declared in c-pragma.h.  */
 
@@ -6417,71 +6418,65 @@ c_parse_error (const char *gmsgid, enum 
    inlining, so we don't have to worry about that.  */
 
 void
-c_warn_unused_result (tree *top_p)
+c_warn_unused_result (gimple_seq seq)
 {
-  tree t = *top_p;
-  tree_stmt_iterator i;
   tree fdecl, ftype;
+  gimple_stmt_iterator i;
 
-  switch (TREE_CODE (t))
+  for (i = gsi_start (seq); !gsi_end_p (i); gsi_next (&i))
     {
-    case STATEMENT_LIST:
-      for (i = tsi_start (*top_p); !tsi_end_p (i); tsi_next (&i))
-	c_warn_unused_result (tsi_stmt_ptr (i));
-      break;
-
-    case COND_EXPR:
-      c_warn_unused_result (&COND_EXPR_THEN (t));
-      c_warn_unused_result (&COND_EXPR_ELSE (t));
-      break;
-    case BIND_EXPR:
-      c_warn_unused_result (&BIND_EXPR_BODY (t));
-      break;
-    case TRY_FINALLY_EXPR:
-    case TRY_CATCH_EXPR:
-      c_warn_unused_result (&TREE_OPERAND (t, 0));
-      c_warn_unused_result (&TREE_OPERAND (t, 1));
-      break;
-    case CATCH_EXPR:
-      c_warn_unused_result (&CATCH_BODY (t));
-      break;
-    case EH_FILTER_EXPR:
-      c_warn_unused_result (&EH_FILTER_FAILURE (t));
-      break;
+      gimple g = gsi_stmt (i);
 
-    case CALL_EXPR:
-      if (TREE_USED (t))
-	break;
-
-      /* This is a naked call, as opposed to a CALL_EXPR nested inside
-	 a MODIFY_EXPR.  All calls whose value is ignored should be
-	 represented like this.  Look for the attribute.  */
-      fdecl = get_callee_fndecl (t);
-      if (fdecl)
-	ftype = TREE_TYPE (fdecl);
-      else
+      switch (GIMPLE_CODE (g))
 	{
-	  ftype = TREE_TYPE (CALL_EXPR_FN (t));
-	  /* Look past pointer-to-function to the function type itself.  */
-	  ftype = TREE_TYPE (ftype);
-	}
+	case GIMPLE_BIND:
+	  c_warn_unused_result (gimple_bind_body (g));
+	  break;
+	case GIMPLE_TRY:
+	  c_warn_unused_result (gimple_try_eval (g));
+	  c_warn_unused_result (gimple_try_cleanup (g));
+	  break;
+	case GIMPLE_CATCH:
+	  c_warn_unused_result (gimple_catch_handler (g));
+	  break;
+	case GIMPLE_EH_FILTER:
+	  c_warn_unused_result (gimple_eh_filter_failure (g));
+	  break;
 
-      if (lookup_attribute ("warn_unused_result", TYPE_ATTRIBUTES (ftype)))
-	{
-	  if (fdecl)
-	    warning (0, "%Hignoring return value of %qD, "
-		     "declared with attribute warn_unused_result",
-		     EXPR_LOCUS (t), fdecl);
+	case GIMPLE_CALL:
+	  if (gimple_call_lhs (g))
+	    break;
+
+	  /* This is a naked call, as opposed to a GIMPLE_CALL with an
+	     LHS.  All calls whose value is ignored should be
+	     represented like this.  Look for the attribute.  */
+	  fdecl = gimple_call_fn (g);
+	  if (TREE_CODE (fdecl) == VAR_DECL)
+	    ftype = TREE_TYPE (fdecl);
 	  else
-	    warning (0, "%Hignoring return value of function "
-		     "declared with attribute warn_unused_result",
-		     EXPR_LOCUS (t));
-	}
-      break;
+	    {
+	      ftype = TREE_TYPE (fdecl);
+	      /* Look past pointer-to-function to the function type itself.  */
+	      ftype = TREE_TYPE (ftype);
+	    }
 
-    default:
-      /* Not a container, not a call, or a call whose value is used.  */
-      break;
+	  if (lookup_attribute ("warn_unused_result", TYPE_ATTRIBUTES (ftype)))
+	    {
+	      if (fdecl)
+		warning (0, "%Hignoring return value of %qD, "
+			 "declared with attribute warn_unused_result",
+			 &GIMPLE_LOCUS (g), fdecl);
+	      else
+		warning (0, "%Hignoring return value of function "
+			 "declared with attribute warn_unused_result",
+			 &GIMPLE_LOCUS (g));
+	    }
+	  break;
+
+	default:
+	  /* Not a container, not a call, or a call whose value is used.  */
+	  break;
+	}
     }
 }
 
Index: c-common.h
===================================================================
--- c-common.h	(revision 126798)
+++ c-common.h	(working copy)
@@ -858,7 +858,7 @@ extern void dump_time_statistics (void);
 
 extern bool c_dump_tree (void *, tree);
 
-extern void c_warn_unused_result (tree *);
+extern void c_warn_unused_result (gimple_seq);
 
 extern void verify_sequence_points (tree);
 
Index: Makefile.in
===================================================================
--- Makefile.in	(revision 126798)
+++ Makefile.in	(working copy)
@@ -1667,7 +1667,7 @@ c-decl.o : c-decl.c $(CONFIG_H) $(SYSTEM
     opts.h $(C_PRAGMA_H) gt-c-decl.h $(CGRAPH_H) $(HASHTAB_H) libfuncs.h \
     except.h $(LANGHOOKS_DEF_H) $(TREE_DUMP_H) $(C_COMMON_H) $(CPPLIB_H) \
     $(DIAGNOSTIC_H) input.h langhooks.h $(TREE_GIMPLE_H) tree-mudflap.h  \
-    pointer-set.h
+    pointer-set.h $(GIMPLE_H)
 c-typeck.o : c-typeck.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
     $(TREE_H) $(C_TREE_H) $(TARGET_H) $(FLAGS_H) intl.h output.h $(EXPR_H) \
     $(RTL_H) toplev.h $(TM_P_H) langhooks.h $(GGC_H) $(TREE_FLOW_H) \
@@ -1730,7 +1730,7 @@ c-common.o : c-common.c $(CONFIG_H) $(SY
 	$(DIAGNOSTIC_H) gt-c-common.h langhooks.h $(VARRAY_H) $(RTL_H) \
 	$(TARGET_H) $(C_TREE_H) tree-iterator.h langhooks.h tree-mudflap.h \
 	intl.h opts.h $(REAL_H) $(CPPLIB_H) $(TREE_INLINE_H) $(HASHTAB_H) \
-	$(BUILTINS_DEF)
+	$(BUILTINS_DEF) $(GIMPLE_H)
 
 c-pretty-print.o : c-pretty-print.c $(C_PRETTY_PRINT_H) \
 	$(C_TREE_H) $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(REAL_H) \
Index: gimple.c
===================================================================
--- gimple.c	(revision 126798)
+++ gimple.c	(working copy)
@@ -334,14 +334,15 @@ gimple_build_asm (const char *string, un
   HANDLER is the exception handler.  */
 
 gimple
-gimple_build_catch (tree types, gimple handler)
+gimple_build_catch (tree types, gimple_seq handler)
 {
   gimple p;
 
   p = ggc_alloc_cleared (sizeof (struct gimple_statement_catch));
   GIMPLE_CODE (p) = GIMPLE_CATCH;
   gimple_catch_set_types (p, types);
-  gimple_catch_set_handler (p, handler);
+  if (handler)
+    gimple_catch_set_handler (p, handler);
 
   return p;
 }
@@ -352,14 +353,15 @@ gimple_build_catch (tree types, gimple h
    FAILURE is the filter's failure action.  */
 
 gimple
-gimple_build_eh_filter (tree types, gimple failure)
+gimple_build_eh_filter (tree types, gimple_seq failure)
 {
   gimple p;
 
   p = ggc_alloc_cleared (sizeof (struct gimple_statement_eh_filter));
   GIMPLE_CODE (p) = GIMPLE_EH_FILTER;
   gimple_eh_filter_set_types (p, types);
-  gimple_eh_filter_set_failure (p, failure);
+  if (failure)
+    gimple_eh_filter_set_failure (p, failure);
 
   return p;
 }
@@ -865,7 +867,7 @@ walk_tuple_ops (gimple gs, walk_tree_fn 
 
     case GIMPLE_CATCH:
       WALKIT (gimple_catch_types (gs));
-      walk_tuple_ops (gimple_catch_handler (gs), func, data, pset);
+      walk_seq_ops (gimple_catch_handler (gs), func, data, pset);
       break;
 
     case GIMPLE_COND:
@@ -877,7 +879,7 @@ walk_tuple_ops (gimple gs, walk_tree_fn 
 
     case GIMPLE_EH_FILTER:
       WALKIT (gimple_eh_filter_types (gs));
-      walk_tuple_ops (gimple_eh_filter_failure (gs), func, data, pset);
+      walk_seq_ops (gimple_eh_filter_failure (gs), func, data, pset);
       break;
 
     case GIMPLE_GOTO:
Index: gimple.h
===================================================================
--- gimple.h	(revision 126798)
+++ gimple.h	(working copy)
@@ -148,7 +148,7 @@ struct gimple_statement_catch GTY(())
 {
   struct gimple_statement_base base;
   tree types;
-  gimple handler;
+  gimple_seq handler;
 };
 
 /* GIMPLE_EH_FILTER */
@@ -158,7 +158,7 @@ struct gimple_statement_eh_filter GTY(()
   /* Filter types.  */
   tree types;
   /* Failure actions.  */
-  gimple failure;
+  gimple_seq failure;
 };
 
 /* GIMPLE_LABEL */
@@ -405,8 +405,8 @@ extern gimple gimple_build_nop (void);
 extern gimple gimple_build_bind (tree, gimple_seq);
 extern gimple gimple_build_asm (const char *, unsigned, unsigned, unsigned,
                                 ...);
-extern gimple gimple_build_catch (tree, gimple);
-extern gimple gimple_build_eh_filter (tree, gimple);
+extern gimple gimple_build_catch (tree, gimple_seq);
+extern gimple gimple_build_eh_filter (tree, gimple_seq);
 extern gimple gimple_build_try (gimple_seq, gimple_seq, unsigned int);
 extern gimple gimple_build_phi (unsigned, unsigned, tree, ...);
 extern gimple gimple_build_resx (int);
@@ -894,7 +894,7 @@ gimple_catch_types (gimple gs)
  return gs->gimple_catch.types;
 }
 
-static inline gimple
+static inline gimple_seq
 gimple_catch_handler (gimple gs)
 {
   GIMPLE_CHECK (gs, GIMPLE_CATCH);
@@ -909,7 +909,7 @@ gimple_catch_set_types (gimple gs, tree 
 }
 
 static inline void
-gimple_catch_set_handler (gimple gs, gimple handler)
+gimple_catch_set_handler (gimple gs, gimple_seq handler)
 {
   GIMPLE_CHECK (gs, GIMPLE_CATCH);
   gs->gimple_catch.handler = handler;
@@ -924,7 +924,7 @@ gimple_eh_filter_types (gimple gs)
   return gs->gimple_eh_filter.types;
 }
 
-static inline gimple
+static inline gimple_seq
 gimple_eh_filter_failure (gimple gs)
 {
   GIMPLE_CHECK (gs, GIMPLE_EH_FILTER);
@@ -939,7 +939,7 @@ gimple_eh_filter_set_types (gimple gs, t
 }
 
 static inline void
-gimple_eh_filter_set_failure (gimple gs, gimple failure)
+gimple_eh_filter_set_failure (gimple gs, gimple_seq failure)
 {
   GIMPLE_CHECK (gs, GIMPLE_EH_FILTER);
   gs->gimple_eh_filter.failure = failure;

Re: [tuples] start c_gimple_diagnostics_recursively conversion

[Aldy Hernandez]

I found we weren't getting the function name correctly, because I was
kludgingly trying to mimic get_callee_fndecl(), when I should have
made get_callee_fndecl smarter instead.

Now get_callee_fndecl() works either if you pass it the entire CALL_EXPR
(before gimplification) or if you pass it the function itself (after
gimplification).

I had a testcase all cooked up, but then I found
attr-warn-unused-result.c already exists, plus I'm apparently too stupid
to use dg-warning correctly.  Arghhh..

Committed to trunk.

	* tree.c (get_callee_fndecl): Work when only the CALL_EXPR_FN has
	been passed.
	* c-common.c (c_warn_unused_result): Use get_callee_fndecl.

Index: tree.c
===================================================================
--- tree.c	(revision 126798)
+++ tree.c	(working copy)
@@ -6373,9 +6373,10 @@ decl_type_context (tree decl)
   return NULL_TREE;
 }
 
-/* CALL is a CALL_EXPR.  Return the declaration for the function
-   called, or NULL_TREE if the called function cannot be
-   determined.  */
+/* CALL is either a CALL_EXPR or the function to call (usually the
+   value of CALL_EXPR_FN in the original CALL_EXPR, or the CALL_FN in
+   a GIMPLE_CALL).  Return the declaration for the function called, or
+   NULL_TREE if the called function cannot be determined.  */
 
 tree
 get_callee_fndecl (tree call)
@@ -6385,13 +6386,16 @@ get_callee_fndecl (tree call)
   if (call == error_mark_node)
     return call;
 
-  /* It's invalid to call this function with anything but a
-     CALL_EXPR.  */
-  gcc_assert (TREE_CODE (call) == CALL_EXPR);
-
   /* The first operand to the CALL is the address of the function
      called.  */
-  addr = CALL_EXPR_FN (call);
+  if (TREE_CODE (call) == CALL_EXPR)
+    addr = CALL_EXPR_FN (call);
+  else
+    {
+      /* If we don't have a CALL_EXPR, we must have the function
+	 itself already.  */
+      addr = call;
+    }
 
   STRIP_NOPS (addr);
 
Index: c-common.c
===================================================================
--- c-common.c	(revision 126849)
+++ c-common.c	(working copy)
@@ -6450,12 +6450,12 @@ c_warn_unused_result (gimple_seq seq)
 	  /* This is a naked call, as opposed to a GIMPLE_CALL with an
 	     LHS.  All calls whose value is ignored should be
 	     represented like this.  Look for the attribute.  */
-	  fdecl = gimple_call_fn (g);
-	  if (TREE_CODE (fdecl) == VAR_DECL)
+	  fdecl = get_callee_fndecl (gimple_call_fn (g));
+	  if (fdecl)
 	    ftype = TREE_TYPE (fdecl);
 	  else
 	    {
-	      ftype = TREE_TYPE (fdecl);
+	      ftype = TREE_TYPE (gimple_call_fn (g));
 	      /* Look past pointer-to-function to the function type itself.  */
 	      ftype = TREE_TYPE (ftype);
 	    }

Re: [tuples] start c_gimple_diagnostics_recursively conversion

[Manuel López-Ibáñez]

On 23/07/07, Aldy Hernandez <aldyh@redhat.com> wrote:

> I had a testcase all cooked up, but then I found
> attr-warn-unused-result.c already exists, plus I'm apparently too stupid
> to use dg-warning correctly.  Arghhh..
>

Or dg-warning is too stupid to understand you. ;-) If you tell us what
you want to match, we can tell you what you need to match it.

Cheers,
Manuel.

Re: [tuples] start c_gimple_diagnostics_recursively conversion

[Aldy Hernandez]

> Or dg-warning is too stupid to understand you. ;-) If you tell us what
> you want to match, we can tell you what you need to match it.

I have something as simple as this:

/* { dg-do compile } */

int foo() __attribute__((warn_unused_result));

void
bar()
{
  foo(); /* { dg-warning "ignoring return value of" } */
}

And the log says something like this:

/source/gimple-tuples-branch/gcc/testsuite/gcc.dg/gimple/call3.c: In function 'bar':
cc1: warning: ignoring return value of 'foo', declared with attribute warn_unused_result
output is:
/source/gimple-tuples-branch/gcc/testsuite/gcc.dg/gimple/call3.c: In function 'bar':
cc1: warning: ignoring return value of 'foo', declared with attribute warn_unused_result

FAIL: gcc.dg/gimple/call3.c  (test for warnings, line 8)
FAIL: gcc.dg/gimple/call3.c (test for excess errors)
Excess errors:
cc1: warning: ignoring return value of 'foo', declared with attribute warn_unused_result

---------------

As we can see, the warning is there.  I have no idea why dg-warning can't
match it.  I'm avoiding doing the obvious-- because no good can come of
debugging dejagnu: it's plain evil.

Aldy

Re: [tuples] start c_gimple_diagnostics_recursively conversion

[Aldy Hernandez]

> Committed to trunk.

I keep saying that word.  I do not think it means what I think it means.

Ok, from now on, everyone can assume every time I say trunk I mean
tuples if [tuples] is in the subject line.  My brain is obviously wired
to type t-r-u-n-k when prepended by "committed to".

Aldy

Re: [tuples] start c_gimple_diagnostics_recursively conversion

[Manuel López-Ibáñez]

On 24/07/07, Aldy Hernandez <aldyh@redhat.com> wrote:
> > Or dg-warning is too stupid to understand you. ;-) If you tell us what
> > you want to match, we can tell you what you need to match it.
>
> I have something as simple as this:
>
> /* { dg-do compile } */
>
> int foo() __attribute__((warn_unused_result));
>
> void
> bar()
> {
>   foo(); /* { dg-warning "ignoring return value of" } */
> }
>
> And the log says something like this:
>
> /source/gimple-tuples-branch/gcc/testsuite/gcc.dg/gimple/call3.c: In function 'bar':
> cc1: warning: ignoring return value of 'foo', declared with attribute warn_unused_result
> output is:
> /source/gimple-tuples-branch/gcc/testsuite/gcc.dg/gimple/call3.c: In function 'bar':
> cc1: warning: ignoring return value of 'foo', declared with attribute warn_unused_result
>
> FAIL: gcc.dg/gimple/call3.c  (test for warnings, line 8)
> FAIL: gcc.dg/gimple/call3.c (test for excess errors)
> Excess errors:
> cc1: warning: ignoring return value of 'foo', declared with attribute warn_unused_result
>
> ---------------
>
> As we can see, the warning is there.  I have no idea why dg-warning can't
> match it.  I'm avoiding doing the obvious-- because no good can come of
> debugging dejagnu: it's plain evil.
>

dg-warning expects this output:
/source/gimple-tuples-branch/gcc/testsuite/gcc.dg/gimple/call3.c:8:
warning:  ignoring return value of 'foo', declared with attribute
warn_unused_result

How is that warning generated? Are you using warning(0, "ignoring
return value of 'foo', declared with attribute warn_unused_result") ?

If you cannot change the output, then you may have more luck using
/* dg-warning "ignoring return value of" "" { target *-*-* } 0 } */
or if that still fails using:
/* dg-message "warning: ignoring return value of" "" { target *-*-* } 0 } */

Yes, dejagnu is plain evil or maybe it is lacking some love and care. Or both.

Cheers,

Manuel.

Re: [tuples] start c_gimple_diagnostics_recursively conversion

[Diego Novillo]

On 7/23/07 6:58 PM, Aldy Hernandez wrote:

> Index: c-common.c
> ===================================================================
> --- c-common.c	(revision 126849)
> +++ c-common.c	(working copy)
> @@ -6450,12 +6450,12 @@ c_warn_unused_result (gimple_seq seq)
>  	  /* This is a naked call, as opposed to a GIMPLE_CALL with an
>  	     LHS.  All calls whose value is ignored should be
>  	     represented like this.  Look for the attribute.  */
> -	  fdecl = gimple_call_fn (g);
> -	  if (TREE_CODE (fdecl) == VAR_DECL)
> +	  fdecl = get_callee_fndecl (gimple_call_fn (g));

get_callee_fndecl should not be needed in GIMPLE.  When we are
gimplifying we should set gimple_call_fn() to the right DECL (so we call
get_callee_fndecl only once in gimplify_call_expr).

> +	  if (fdecl)

This test should then become 'if (DECL_P (gimple_call_fn (g)))'

Re: [tuples] start c_gimple_diagnostics_recursively conversion

[Aldy Hernandez]

> get_callee_fndecl should not be needed in GIMPLE.  When we are
> gimplifying we should set gimple_call_fn() to the right DECL (so we call
> get_callee_fndecl only once in gimplify_call_expr).

Ok, makes sense.

> > +	  if (fdecl)
> 
> This test should then become 'if (DECL_P (gimple_call_fn (g)))'

Actually, it should be 'if (TREE_CODE (fdecl) == FUNCTION_DECL)'.  See
patch.  We either have FUNCTION_DECL (when get_callee_fndecl returned
something), or we have the VAR_DECL (when we are calling a function
through a pointer).

Is the patch below ok?

	* tree.c (get_callee_fndecl): Revert previous change.
	* gimplify.c (gimplify_call_expr): Use result from
	get_callee_fndecl if available.
	* c-common.c (c_warn_unused_result): Do not use
	get_callee_fndecl.
	Add assertion.

Index: tree.c
===================================================================
--- tree.c	(revision 126860)
+++ tree.c	(working copy)
@@ -6373,10 +6373,9 @@ decl_type_context (tree decl)
   return NULL_TREE;
 }
 
-/* CALL is either a CALL_EXPR or the function to call (usually the
-   value of CALL_EXPR_FN in the original CALL_EXPR, or the CALL_FN in
-   a GIMPLE_CALL).  Return the declaration for the function called, or
-   NULL_TREE if the called function cannot be determined.  */
+/* CALL is a CALL_EXPR.  Return the declaration for the function
+   called, or NULL_TREE if the called function cannot be
+   determined.  */
 
 tree
 get_callee_fndecl (tree call)
@@ -6386,16 +6385,13 @@ get_callee_fndecl (tree call)
   if (call == error_mark_node)
     return call;
 
+  /* It's invalid to call this function with anything but a
+     CALL_EXPR.  */
+  gcc_assert (TREE_CODE (call) == CALL_EXPR);
+
   /* The first operand to the CALL is the address of the function
      called.  */
-  if (TREE_CODE (call) == CALL_EXPR)
-    addr = CALL_EXPR_FN (call);
-  else
-    {
-      /* If we don't have a CALL_EXPR, we must have the function
-	 itself already.  */
-      addr = call;
-    }
+  addr = CALL_EXPR_FN (call);
 
   STRIP_NOPS (addr);
 
Index: gimplify.c
===================================================================
--- gimplify.c	(revision 126849)
+++ gimplify.c	(working copy)
@@ -2180,7 +2180,7 @@ gimplify_call_expr (tree *expr_p, gimple
 
   /* Now add the GIMPLE call to PRE_P.  If WANT_VALUE is set, we need
      to create the appropriate temporary for the call's LHS.  */
-  call = gimple_build_call_vec (CALL_EXPR_FN (*expr_p), args);
+  call = gimple_build_call_vec (fndecl ? fndecl : CALL_EXPR_FN (*expr_p), args);
   gimple_add (pre_p, call);
   if (want_value)
     {
Index: c-common.c
===================================================================
--- c-common.c	(revision 126860)
+++ c-common.c	(working copy)
@@ -6450,15 +6450,16 @@ c_warn_unused_result (gimple_seq seq)
 	  /* This is a naked call, as opposed to a GIMPLE_CALL with an
 	     LHS.  All calls whose value is ignored should be
 	     represented like this.  Look for the attribute.  */
-	  fdecl = get_callee_fndecl (gimple_call_fn (g));
-	  if (fdecl)
+	  fdecl = gimple_call_fn (g);
+	  if (TREE_CODE (fdecl) == FUNCTION_DECL)
 	    ftype = TREE_TYPE (fdecl);
 	  else
 	    {
-	      ftype = TREE_TYPE (gimple_call_fn (g));
+	      ftype = TREE_TYPE (fdecl);
 	      /* Look past pointer-to-function to the function type itself.  */
 	      ftype = TREE_TYPE (ftype);
 	    }
+	  gcc_assert (TREE_CODE (ftype) == FUNCTION_TYPE);
 
 	  if (lookup_attribute ("warn_unused_result", TYPE_ATTRIBUTES (ftype)))
 	    {

Re: [tuples] start c_gimple_diagnostics_recursively conversion

[Diego Novillo]

On 7/24/07 11:16 AM, Aldy Hernandez wrote:

> Is the patch below ok?

Yup, thanks.

Re: [tuples] start c_gimple_diagnostics_recursively conversion

[Manuel López-Ibáñez]

On 24/07/07, Aldy Hernandez <aldyh@redhat.com> wrote:
> > Or dg-warning is too stupid to understand you. ;-) If you tell us what
> > you want to match, we can tell you what you need to match it.
>
> And the log says something like this:
>
> /source/gimple-tuples-branch/gcc/testsuite/gcc.dg/gimple/call3.c: In function 'bar':
> cc1: warning: ignoring return value of 'foo', declared with attribute warn_unused_result
> output is:
> /source/gimple-tuples-branch/gcc/testsuite/gcc.dg/gimple/call3.c: In function 'bar':
> cc1: warning: ignoring return value of 'foo', declared with attribute warn_unused_result
[snip]
>
> As we can see, the warning is there.  I have no idea why dg-warning can't
> match it.  I'm avoiding doing the obvious-- because no good can come of
> debugging dejagnu: it's plain evil.

DejaGNU may be evil but in this case it is telling you that you are
introducing a diagnostics regression. The output used to be (4.1.2)

test1.c: In function 'bar':
test1.c:6: warning: ignoring return value of 'foo', declared with
attribute warn_unused_result

I think that in your code:

+               warning (0, "%Hignoring return value of %qD, "
+                        "declared with attribute warn_unused_result",
+                        &GIMPLE_LOCUS (g), fdecl);

&GIMPLE_LOCUS(g) is not doing whatever it should do and that
EXPR_LOCUS (t) used to do.

So, please, don't change the testcase as I suggested above. The
testcase is correct.

Cheers,

Manuel.

Re: [tuples] start c_gimple_diagnostics_recursively conversion

[Aldy Hernandez]

On Tue, Jul 24, 2007 at 11:20:49AM -0400, Diego Novillo wrote:
> On 7/24/07 11:16 AM, Aldy Hernandez wrote:
> 
> > Is the patch below ok?
> 
> Yup, thanks.

[meanwhile, conflicts are resolved from my tree]

Committed.

Thanks.