diff mbox

Fix PR70457 (ICE on incompatible call to built-in pow)

Message ID 1459727027.6430.52.camel@oc8801110288.ibm.com
State New
Headers show

Commit Message

Bill Schmidt April 3, 2016, 11:43 p.m. UTC 
Hi,

PR70457 shows that we ICE if a call to pow() has only one argument, or
more generally doesn't match its expected signature.  This can happen
both during the widen-mult phase and the tree-inline phase.  As
suggested by Jakub, this patch uses gimple_call_combined_fn to test for
this, similarly to what is done elsewhere in tree-ssa-math-opts.c.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu, with no
regressions.  Is this ok for trunk, as well as for backport to 5 and
4.9?

Thanks,
Bill


[gcc]

2016-04-03  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
	    Jakub Jelinek <jakub@redhat.com>

	* tree-inline.c (estimate_num_insn): Use gimple_call_combined_fn
	to ensure a call statement is compatible with a built-in's
	prototype.
	* tree-ssa-math-opts.c (pass_optimize_windening_mul::execute):
	Likewise.

[gcc/testsuite]

2016-04-03  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
	    Jakub Jelinek <jakub@redhat.com>

	* gcc.dg/torture/pr70457.c: New.

Comments

Jakub Jelinek April 4, 2016, 6:57 a.m. UTC | #1 
On Sun, Apr 03, 2016 at 06:43:47PM -0500, Bill Schmidt wrote:
> --- tree-inline.c	(revision 234702)
> +++ tree-inline.c	(working copy)
> @@ -57,8 +57,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "cfgloop.h"
>  #include "builtins.h"
>  #include "tree-chkp.h"
> +#include "case-cfn-macros.h"
>  
> -
>  /* I'm not real happy about this, but we need to handle gimple and
>     non-gimple trees.  */
>  

Please keep the extra empty line above.

> @@ -4070,11 +4070,9 @@ estimate_num_insns (gimple *stmt, eni_weights *wei
>  		/* We canonicalize x * x to pow (x, 2.0) with -ffast-math, so
>  		   specialize the cheap expansion we do here.
>  		   ???  This asks for a more general solution.  */
> -		switch (DECL_FUNCTION_CODE (decl))
> +		switch (gimple_call_combined_fn (stmt))
>  		  {
> -		    case BUILT_IN_POW:
> -		    case BUILT_IN_POWF:
> -		    case BUILT_IN_POWL:
> +  		    CASE_CFN_POW:
>  		      if (TREE_CODE (gimple_call_arg (stmt, 1)) == REAL_CST
>  			  && (real_equal
>  			      (&TREE_REAL_CST (gimple_call_arg (stmt, 1)),

Actually, I haven't been suggesting to use gimple_call_combined_fn,
but gimple_call_builtin_p.
Here using gimple_call_combined_fn doesn't make much sense, because
it is in code guarded with:
        if (gimple_call_internal_p (stmt))
          return 0;
        else if ((decl = gimple_call_fndecl (stmt))
                 && DECL_BUILT_IN (decl))
          {
...
            else if (DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL)
              {
Thus, internal functions don't make this spot at all.
So, either replace the DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL
with gimple_builtin_call_p (stmt, BUILT_IN_NORMAL); or call
gimple_builtin_call_types_compatible_p.

> Index: tree-ssa-math-opts.c
> ===================================================================
> --- tree-ssa-math-opts.c	(revision 234702)
> +++ tree-ssa-math-opts.c	(working copy)
> @@ -3829,11 +3829,9 @@ pass_optimize_widening_mul::execute (function *fun
>  	      if (fndecl
>  		  && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
>  		{
> -		  switch (DECL_FUNCTION_CODE (fndecl))
> +		  switch (gimple_call_combined_fn (stmt))
>  		    {
> -		      case BUILT_IN_POWF:
> -		      case BUILT_IN_POW:
> -		      case BUILT_IN_POWL:
> +		      CASE_CFN_POW:
>  			if (TREE_CODE (gimple_call_arg (stmt, 1)) == REAL_CST
>  			    && real_equal
>  			         (&TREE_REAL_CST (gimple_call_arg (stmt, 1)),
> 

And similarly here.

	Jakub
Bill Schmidt April 4, 2016, 12:17 p.m. UTC | #2 
OK, sorry for misreading the note.  This is exactly what I've done for
the GCC 5 and GCC 4.9 versions, so I'll update the GCC 6 version to do
the same.

Bill

On Mon, 2016-04-04 at 08:57 +0200, Jakub Jelinek wrote:
> On Sun, Apr 03, 2016 at 06:43:47PM -0500, Bill Schmidt wrote:
> > --- tree-inline.c	(revision 234702)
> > +++ tree-inline.c	(working copy)
> > @@ -57,8 +57,8 @@ along with GCC; see the file COPYING3.  If not see
> >  #include "cfgloop.h"
> >  #include "builtins.h"
> >  #include "tree-chkp.h"
> > +#include "case-cfn-macros.h"
> >  
> > -
> >  /* I'm not real happy about this, but we need to handle gimple and
> >     non-gimple trees.  */
> >  
> 
> Please keep the extra empty line above.
> 
> > @@ -4070,11 +4070,9 @@ estimate_num_insns (gimple *stmt, eni_weights *wei
> >  		/* We canonicalize x * x to pow (x, 2.0) with -ffast-math, so
> >  		   specialize the cheap expansion we do here.
> >  		   ???  This asks for a more general solution.  */
> > -		switch (DECL_FUNCTION_CODE (decl))
> > +		switch (gimple_call_combined_fn (stmt))
> >  		  {
> > -		    case BUILT_IN_POW:
> > -		    case BUILT_IN_POWF:
> > -		    case BUILT_IN_POWL:
> > +  		    CASE_CFN_POW:
> >  		      if (TREE_CODE (gimple_call_arg (stmt, 1)) == REAL_CST
> >  			  && (real_equal
> >  			      (&TREE_REAL_CST (gimple_call_arg (stmt, 1)),
> 
> Actually, I haven't been suggesting to use gimple_call_combined_fn,
> but gimple_call_builtin_p.
> Here using gimple_call_combined_fn doesn't make much sense, because
> it is in code guarded with:
>         if (gimple_call_internal_p (stmt))
>           return 0;
>         else if ((decl = gimple_call_fndecl (stmt))
>                  && DECL_BUILT_IN (decl))
>           {
> ...
>             else if (DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL)
>               {
> Thus, internal functions don't make this spot at all.
> So, either replace the DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL
> with gimple_builtin_call_p (stmt, BUILT_IN_NORMAL); or call
> gimple_builtin_call_types_compatible_p.
> 
> > Index: tree-ssa-math-opts.c
> > ===================================================================
> > --- tree-ssa-math-opts.c	(revision 234702)
> > +++ tree-ssa-math-opts.c	(working copy)
> > @@ -3829,11 +3829,9 @@ pass_optimize_widening_mul::execute (function *fun
> >  	      if (fndecl
> >  		  && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> >  		{
> > -		  switch (DECL_FUNCTION_CODE (fndecl))
> > +		  switch (gimple_call_combined_fn (stmt))
> >  		    {
> > -		      case BUILT_IN_POWF:
> > -		      case BUILT_IN_POW:
> > -		      case BUILT_IN_POWL:
> > +		      CASE_CFN_POW:
> >  			if (TREE_CODE (gimple_call_arg (stmt, 1)) == REAL_CST
> >  			    && real_equal
> >  			         (&TREE_REAL_CST (gimple_call_arg (stmt, 1)),
> > 
> 
> And similarly here.
> 
> 	Jakub
>
diff mbox

Patch

Index: testsuite/gcc.dg/torture/pr70457.c
===================================================================
--- testsuite/gcc.dg/torture/pr70457.c	(revision 0)
+++ testsuite/gcc.dg/torture/pr70457.c	(working copy)
@@ -0,0 +1,32 @@ 
+/* { dg-do compile } */
+
+/* This formerly ICEd when trying to expand pow as a built-in with
+   the wrong number of arguments.  */
+
+extern double pow (double __x, double __y) __attribute__ ((__nothrow__ , __leaf__));
+extern double __pow (double __x, double __y) __attribute__ ((__nothrow__ , __leaf__));
+
+typedef int int64_t __attribute__ ((__mode__ (__DI__)));
+
+typedef struct {
+  int64_t data;
+  int tag;
+} Object;
+
+extern Object Make_Flonum (double);
+extern Object P_Pow (Object, Object);
+
+Object General_Function (Object x, Object y, double (*fun)()) {
+  double d, ret;
+
+  d = 1.0;
+
+  if (y.tag >> 1)
+    ret = (*fun) (d);
+  else
+    ret = (*fun) (d, 0.0);
+
+  return Make_Flonum (ret);
+}
+
+Object P_Pow (Object x, Object y) { return General_Function (x, y, pow); }
Index: tree-inline.c
===================================================================
--- tree-inline.c	(revision 234702)
+++ tree-inline.c	(working copy)
@@ -57,8 +57,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "cfgloop.h"
 #include "builtins.h"
 #include "tree-chkp.h"
+#include "case-cfn-macros.h"
 
-
 /* I'm not real happy about this, but we need to handle gimple and
    non-gimple trees.  */
 
@@ -4070,11 +4070,9 @@  estimate_num_insns (gimple *stmt, eni_weights *wei
 		/* We canonicalize x * x to pow (x, 2.0) with -ffast-math, so
 		   specialize the cheap expansion we do here.
 		   ???  This asks for a more general solution.  */
-		switch (DECL_FUNCTION_CODE (decl))
+		switch (gimple_call_combined_fn (stmt))
 		  {
-		    case BUILT_IN_POW:
-		    case BUILT_IN_POWF:
-		    case BUILT_IN_POWL:
+  		    CASE_CFN_POW:
 		      if (TREE_CODE (gimple_call_arg (stmt, 1)) == REAL_CST
 			  && (real_equal
 			      (&TREE_REAL_CST (gimple_call_arg (stmt, 1)),
Index: tree-ssa-math-opts.c
===================================================================
--- tree-ssa-math-opts.c	(revision 234702)
+++ tree-ssa-math-opts.c	(working copy)
@@ -3829,11 +3829,9 @@  pass_optimize_widening_mul::execute (function *fun
 	      if (fndecl
 		  && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
 		{
-		  switch (DECL_FUNCTION_CODE (fndecl))
+		  switch (gimple_call_combined_fn (stmt))
 		    {
-		      case BUILT_IN_POWF:
-		      case BUILT_IN_POW:
-		      case BUILT_IN_POWL:
+		      CASE_CFN_POW:
 			if (TREE_CODE (gimple_call_arg (stmt, 1)) == REAL_CST
 			    && real_equal
 			         (&TREE_REAL_CST (gimple_call_arg (stmt, 1)),