[PATCH][C++] Improve compile-time by ordering expensive checks last

[PATCH][C++] Improve compile-time by ordering expensive checks last

[Richard Biener]

Two cases from a -fsynax-only tramp3d callgrind profile.

Bootstrapped / tested on x86_64-unknown-linux-gnu.

OK for trunk?

Richard.

2019-04-16  Richard Biener  <rguenther@suse.de>

	cp/
	* call.c (null_ptr_cst_p): Order checks according to expensiveness.
	(conversion_null_warnings): Likewise.

Index: gcc/cp/call.c
===================================================================
--- gcc/cp/call.c	(revision 270387)
+++ gcc/cp/call.c	(working copy)
@@ -541,11 +541,11 @@ null_ptr_cst_p (tree t)
       STRIP_ANY_LOCATION_WRAPPER (t);
 
       /* Core issue 903 says only literal 0 is a null pointer constant.  */
-      if (TREE_CODE (type) == INTEGER_TYPE
-	  && !char_type_p (type)
-	  && TREE_CODE (t) == INTEGER_CST
+      if (TREE_CODE (t) == INTEGER_CST
+	  && !TREE_OVERFLOW (t)
+	  && TREE_CODE (type) == INTEGER_TYPE
 	  && integer_zerop (t)
-	  && !TREE_OVERFLOW (t))
+	  && !char_type_p (type))
 	return true;
     }
   else if (CP_INTEGRAL_TYPE_P (type))
@@ -6882,8 +6882,8 @@ conversion_null_warnings (tree totype, t
     }
   /* Handle zero as null pointer warnings for cases other
      than EQ_EXPR and NE_EXPR */
-  else if (null_ptr_cst_p (expr) &&
-	   (TYPE_PTR_OR_PTRMEM_P (totype) || NULLPTR_TYPE_P (totype)))
+  else if ((TYPE_PTR_OR_PTRMEM_P (totype) || NULLPTR_TYPE_P (totype))
+	   && null_ptr_cst_p (expr))
     {
       location_t loc = get_location_for_expr_unwinding_for_system_header (expr);
       maybe_warn_zero_as_null_pointer_constant (expr, loc);

Re: [PATCH][C++] Improve compile-time by ordering expensive checks last

[Richard Biener]

On Tue, 16 Apr 2019, Richard Biener wrote:

> 
> Two cases from a -fsynax-only tramp3d callgrind profile.

Amended by two others.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

OK?

Thanks,
Richard.

2019-04-17  Richard Biener  <rguenther@suse.de>

	cp/
	* call.c (null_ptr_cst_p): Order checks according to expensiveness.
	(conversion_null_warnings): Likewise.
	* typeck.c (same_type_ignoring_top_level_qualifiers_p): Return
	early if type1 == type2.

Index: gcc/cp/call.c
===================================================================
--- gcc/cp/call.c	(revision 270407)
+++ gcc/cp/call.c	(working copy)
@@ -541,11 +541,11 @@ null_ptr_cst_p (tree t)
       STRIP_ANY_LOCATION_WRAPPER (t);
 
       /* Core issue 903 says only literal 0 is a null pointer constant.  */
-      if (TREE_CODE (type) == INTEGER_TYPE
-	  && !char_type_p (type)
-	  && TREE_CODE (t) == INTEGER_CST
+      if (TREE_CODE (t) == INTEGER_CST
+	  && !TREE_OVERFLOW (t)
+	  && TREE_CODE (type) == INTEGER_TYPE
 	  && integer_zerop (t)
-	  && !TREE_OVERFLOW (t))
+	  && !char_type_p (type))
 	return true;
     }
   else if (CP_INTEGRAL_TYPE_P (type))
@@ -6844,8 +6844,9 @@ static void
 conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
 {
   /* Issue warnings about peculiar, but valid, uses of NULL.  */
-  if (null_node_p (expr) && TREE_CODE (totype) != BOOLEAN_TYPE
-      && ARITHMETIC_TYPE_P (totype))
+  if (TREE_CODE (totype) != BOOLEAN_TYPE
+      && ARITHMETIC_TYPE_P (totype)
+      && null_node_p (expr))
     {
       location_t loc = get_location_for_expr_unwinding_for_system_header (expr);
       if (fn)
@@ -6882,8 +6883,8 @@ conversion_null_warnings (tree totype, t
     }
   /* Handle zero as null pointer warnings for cases other
      than EQ_EXPR and NE_EXPR */
-  else if (null_ptr_cst_p (expr) &&
-	   (TYPE_PTR_OR_PTRMEM_P (totype) || NULLPTR_TYPE_P (totype)))
+  else if ((TYPE_PTR_OR_PTRMEM_P (totype) || NULLPTR_TYPE_P (totype))
+	   && null_ptr_cst_p (expr))
     {
       location_t loc = get_location_for_expr_unwinding_for_system_header (expr);
       maybe_warn_zero_as_null_pointer_constant (expr, loc);
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 270407)
+++ gcc/cp/typeck.c	(working copy)
@@ -1508,6 +1508,8 @@ same_type_ignoring_top_level_qualifiers_
 {
   if (type1 == error_mark_node || type2 == error_mark_node)
     return false;
+  if (type1 == type2)
+    return true;
 
   type1 = cp_build_qualified_type (type1, TYPE_UNQUALIFIED);
   type2 = cp_build_qualified_type (type2, TYPE_UNQUALIFIED);

Re: [PATCH][C++] Improve compile-time by ordering expensive checks last

[Jason Merrill]

OK.

On Wed, Apr 17, 2019 at 6:44 AM Richard Biener <rguenther@suse.de> wrote:
>
> On Tue, 16 Apr 2019, Richard Biener wrote:
>
> >
> > Two cases from a -fsynax-only tramp3d callgrind profile.
>
> Amended by two others.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> OK?
>
> Thanks,
> Richard.
>
> 2019-04-17  Richard Biener  <rguenther@suse.de>
>
>         cp/
>         * call.c (null_ptr_cst_p): Order checks according to expensiveness.
>         (conversion_null_warnings): Likewise.
>         * typeck.c (same_type_ignoring_top_level_qualifiers_p): Return
>         early if type1 == type2.
>
> Index: gcc/cp/call.c
> ===================================================================
> --- gcc/cp/call.c       (revision 270407)
> +++ gcc/cp/call.c       (working copy)
> @@ -541,11 +541,11 @@ null_ptr_cst_p (tree t)
>        STRIP_ANY_LOCATION_WRAPPER (t);
>
>        /* Core issue 903 says only literal 0 is a null pointer constant.  */
> -      if (TREE_CODE (type) == INTEGER_TYPE
> -         && !char_type_p (type)
> -         && TREE_CODE (t) == INTEGER_CST
> +      if (TREE_CODE (t) == INTEGER_CST
> +         && !TREE_OVERFLOW (t)
> +         && TREE_CODE (type) == INTEGER_TYPE
>           && integer_zerop (t)
> -         && !TREE_OVERFLOW (t))
> +         && !char_type_p (type))
>         return true;
>      }
>    else if (CP_INTEGRAL_TYPE_P (type))
> @@ -6844,8 +6844,9 @@ static void
>  conversion_null_warnings (tree totype, tree expr, tree fn, int argnum)
>  {
>    /* Issue warnings about peculiar, but valid, uses of NULL.  */
> -  if (null_node_p (expr) && TREE_CODE (totype) != BOOLEAN_TYPE
> -      && ARITHMETIC_TYPE_P (totype))
> +  if (TREE_CODE (totype) != BOOLEAN_TYPE
> +      && ARITHMETIC_TYPE_P (totype)
> +      && null_node_p (expr))
>      {
>        location_t loc = get_location_for_expr_unwinding_for_system_header (expr);
>        if (fn)
> @@ -6882,8 +6883,8 @@ conversion_null_warnings (tree totype, t
>      }
>    /* Handle zero as null pointer warnings for cases other
>       than EQ_EXPR and NE_EXPR */
> -  else if (null_ptr_cst_p (expr) &&
> -          (TYPE_PTR_OR_PTRMEM_P (totype) || NULLPTR_TYPE_P (totype)))
> +  else if ((TYPE_PTR_OR_PTRMEM_P (totype) || NULLPTR_TYPE_P (totype))
> +          && null_ptr_cst_p (expr))
>      {
>        location_t loc = get_location_for_expr_unwinding_for_system_header (expr);
>        maybe_warn_zero_as_null_pointer_constant (expr, loc);
> Index: gcc/cp/typeck.c
> ===================================================================
> --- gcc/cp/typeck.c     (revision 270407)
> +++ gcc/cp/typeck.c     (working copy)
> @@ -1508,6 +1508,8 @@ same_type_ignoring_top_level_qualifiers_
>  {
>    if (type1 == error_mark_node || type2 == error_mark_node)
>      return false;
> +  if (type1 == type2)
> +    return true;
>
>    type1 = cp_build_qualified_type (type1, TYPE_UNQUALIFIED);
>    type2 = cp_build_qualified_type (type2, TYPE_UNQUALIFIED);

Re: [PATCH][C++] Improve compile-time by ordering expensive checks last

[Rainer Orth]

Hi Richard,

> On Tue, 16 Apr 2019, Richard Biener wrote:
>
>> 
>> Two cases from a -fsynax-only tramp3d callgrind profile.
>
> Amended by two others.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> OK?
>
> Thanks,
> Richard.
>
> 2019-04-17  Richard Biener  <rguenther@suse.de>
>
> 	cp/
> 	* call.c (null_ptr_cst_p): Order checks according to expensiveness.
> 	(conversion_null_warnings): Likewise.
> 	* typeck.c (same_type_ignoring_top_level_qualifiers_p): Return
> 	early if type1 == type2.

this patch caused

+XPASS: g++.dg/warn/Wunused-var-35.C  -std=gnu++98 bug (test for warnings, line 14)

First seen on i386-pc-solaris2.11 and sparc-sun-solaris2.11, according
to gcc-testresults everywhere.  Confirmed by reverting the patch locally
and re-testing the affected testcase.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

[PATCH] Fix -Wunused-var-35.C (was Re: [PATCH][C++] Improve compile-time by ordering expensive checks last)

[Jakub Jelinek]

On Thu, Apr 25, 2019 at 01:09:00PM +0200, Rainer Orth wrote:
> > 	cp/
> > 	* call.c (null_ptr_cst_p): Order checks according to expensiveness.
> > 	(conversion_null_warnings): Likewise.
> > 	* typeck.c (same_type_ignoring_top_level_qualifiers_p): Return
> > 	early if type1 == type2.
> 
> this patch caused
> 
> +XPASS: g++.dg/warn/Wunused-var-35.C  -std=gnu++98 bug (test for warnings, line 14)
> 
> First seen on i386-pc-solaris2.11 and sparc-sun-solaris2.11, according
> to gcc-testresults everywhere.  Confirmed by reverting the patch locally
> and re-testing the affected testcase.

I can reproduce that too, seems to be the
@@ -6896,8 +6897,8 @@
     }
   /* Handle zero as null pointer warnings for cases other
      than EQ_EXPR and NE_EXPR */
-  else if (null_ptr_cst_p (expr) &&
-	   (TYPE_PTR_OR_PTRMEM_P (totype) || NULLPTR_TYPE_P (totype)))
+  else if ((TYPE_PTR_OR_PTRMEM_P (totype) || NULLPTR_TYPE_P (totype))
+	   && null_ptr_cst_p (expr))
     {
       location_t loc = get_location_for_expr_unwinding_for_system_header (expr);
       maybe_warn_zero_as_null_pointer_constant (expr, loc);
hunk.  The Wunused-var-35.C patch hasn't been posted to gcc-patches, but
judging from the corresponding PR, I'd say the right thing is below.

Regtested on x86_64-linux and i686-linux, ok for trunk?

2019-04-25  Jakub Jelinek  <jakub@redhat.com>

	PR c++/44648
	* g++.dg/warn/Wunused-var-35.C: Remove xfail.

--- gcc/testsuite/g++.dg/warn/Wunused-var-35.C.jj	2019-02-04 09:44:31.365413407 +0100
+++ gcc/testsuite/g++.dg/warn/Wunused-var-35.C	2019-04-25 13:24:49.717065815 +0200
@@ -11,9 +11,8 @@ int main()
   else
     return 1;
 
-  if (const bool b2 = 1) // { dg-warning "\\\[-Wunused-variable\\\]" "bug" { xfail c++98_only } }
+  if (const bool b2 = 1) // { dg-warning "\\\[-Wunused-variable\\\]" }
     return 0;
   else
     return 1;
 }
-


	Jakub

Re: [PATCH] Fix -Wunused-var-35.C (was Re: [PATCH][C++] Improve compile-time by ordering expensive checks last)

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 2325 bytes --]

On Thu, 25 Apr 2019, Jakub Jelinek wrote:

> On Thu, Apr 25, 2019 at 01:09:00PM +0200, Rainer Orth wrote:
> > > 	cp/
> > > 	* call.c (null_ptr_cst_p): Order checks according to expensiveness.
> > > 	(conversion_null_warnings): Likewise.
> > > 	* typeck.c (same_type_ignoring_top_level_qualifiers_p): Return
> > > 	early if type1 == type2.
> > 
> > this patch caused
> > 
> > +XPASS: g++.dg/warn/Wunused-var-35.C  -std=gnu++98 bug (test for warnings, line 14)
> > 
> > First seen on i386-pc-solaris2.11 and sparc-sun-solaris2.11, according
> > to gcc-testresults everywhere.  Confirmed by reverting the patch locally
> > and re-testing the affected testcase.
> 
> I can reproduce that too, seems to be the
> @@ -6896,8 +6897,8 @@
>      }
>    /* Handle zero as null pointer warnings for cases other
>       than EQ_EXPR and NE_EXPR */
> -  else if (null_ptr_cst_p (expr) &&
> -	   (TYPE_PTR_OR_PTRMEM_P (totype) || NULLPTR_TYPE_P (totype)))
> +  else if ((TYPE_PTR_OR_PTRMEM_P (totype) || NULLPTR_TYPE_P (totype))
> +	   && null_ptr_cst_p (expr))
>      {
>        location_t loc = get_location_for_expr_unwinding_for_system_header (expr);
>        maybe_warn_zero_as_null_pointer_constant (expr, loc);
> hunk.  The Wunused-var-35.C patch hasn't been posted to gcc-patches, but
> judging from the corresponding PR, I'd say the right thing is below.
> 
> Regtested on x86_64-linux and i686-linux, ok for trunk?

The patch would be obvious but I wonder how the above can change 
anything...  (but I can reproduce with even only that change)

Richard.

> 2019-04-25  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/44648
> 	* g++.dg/warn/Wunused-var-35.C: Remove xfail.
> 
> --- gcc/testsuite/g++.dg/warn/Wunused-var-35.C.jj	2019-02-04 09:44:31.365413407 +0100
> +++ gcc/testsuite/g++.dg/warn/Wunused-var-35.C	2019-04-25 13:24:49.717065815 +0200
> @@ -11,9 +11,8 @@ int main()
>    else
>      return 1;
>  
> -  if (const bool b2 = 1) // { dg-warning "\\\[-Wunused-variable\\\]" "bug" { xfail c++98_only } }
> +  if (const bool b2 = 1) // { dg-warning "\\\[-Wunused-variable\\\]" }
>      return 0;
>    else
>      return 1;
>  }
> -
> 
> 
> 	Jakub
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG NÌrnberg)

Re: [PATCH] Fix -Wunused-var-35.C (was Re: [PATCH][C++] Improve compile-time by ordering expensive checks last)

[Jakub Jelinek]

On Thu, Apr 25, 2019 at 01:39:02PM +0200, Richard Biener wrote:
> > Regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> The patch would be obvious but I wonder how the above can change 
> anything...  (but I can reproduce with even only that change)

Haven't stepped through it in a debugger, but I bet it is the
fold_non_dependent_expr call in null_ptr_cst_p which is done for C++98 only.
That will mark it as TREE_USED in the end most likely and the
earlier fix for the PR44648 issues, r249083, was also about not trying to
fold the stuff early.

> > 2019-04-25  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR c++/44648
> > 	* g++.dg/warn/Wunused-var-35.C: Remove xfail.
> > 
> > --- gcc/testsuite/g++.dg/warn/Wunused-var-35.C.jj	2019-02-04 09:44:31.365413407 +0100
> > +++ gcc/testsuite/g++.dg/warn/Wunused-var-35.C	2019-04-25 13:24:49.717065815 +0200
> > @@ -11,9 +11,8 @@ int main()
> >    else
> >      return 1;
> >  
> > -  if (const bool b2 = 1) // { dg-warning "\\\[-Wunused-variable\\\]" "bug" { xfail c++98_only } }
> > +  if (const bool b2 = 1) // { dg-warning "\\\[-Wunused-variable\\\]" }
> >      return 0;
> >    else
> >      return 1;
> >  }
> > -

	Jakub

Re: [PATCH] Fix -Wunused-var-35.C (was Re: [PATCH][C++] Improve compile-time by ordering expensive checks last)

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]

On Thu, 25 Apr 2019, Jakub Jelinek wrote:

> On Thu, Apr 25, 2019 at 01:39:02PM +0200, Richard Biener wrote:
> > > Regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > The patch would be obvious but I wonder how the above can change 
> > anything...  (but I can reproduce with even only that change)
> 
> Haven't stepped through it in a debugger, but I bet it is the
> fold_non_dependent_expr call in null_ptr_cst_p which is done for C++98 only.
> That will mark it as TREE_USED in the end most likely and the
> earlier fix for the PR44648 issues, r249083, was also about not trying to
> fold the stuff early.

Ah, OK, that makes "sense" (well, it doesn't really make sense for
a function called null_ptr_cst_p to behave this way...)

Richard.

> > > 2019-04-25  Jakub Jelinek  <jakub@redhat.com>
> > > 
> > > 	PR c++/44648
> > > 	* g++.dg/warn/Wunused-var-35.C: Remove xfail.
> > > 
> > > --- gcc/testsuite/g++.dg/warn/Wunused-var-35.C.jj	2019-02-04 09:44:31.365413407 +0100
> > > +++ gcc/testsuite/g++.dg/warn/Wunused-var-35.C	2019-04-25 13:24:49.717065815 +0200
> > > @@ -11,9 +11,8 @@ int main()
> > >    else
> > >      return 1;
> > >  
> > > -  if (const bool b2 = 1) // { dg-warning "\\\[-Wunused-variable\\\]" "bug" { xfail c++98_only } }
> > > +  if (const bool b2 = 1) // { dg-warning "\\\[-Wunused-variable\\\]" }
> > >      return 0;
> > >    else
> > >      return 1;
> > >  }
> > > -
> 
> 	Jakub
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG NÌrnberg)

Re: [PATCH] Fix -Wunused-var-35.C (was Re: [PATCH][C++] Improve compile-time by ordering expensive checks last)

[Jason Merrill]

OK.

On Thu, Apr 25, 2019 at 7:27 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Apr 25, 2019 at 01:09:00PM +0200, Rainer Orth wrote:
> > >     cp/
> > >     * call.c (null_ptr_cst_p): Order checks according to expensiveness.
> > >     (conversion_null_warnings): Likewise.
> > >     * typeck.c (same_type_ignoring_top_level_qualifiers_p): Return
> > >     early if type1 == type2.
> >
> > this patch caused
> >
> > +XPASS: g++.dg/warn/Wunused-var-35.C  -std=gnu++98 bug (test for warnings, line 14)
> >
> > First seen on i386-pc-solaris2.11 and sparc-sun-solaris2.11, according
> > to gcc-testresults everywhere.  Confirmed by reverting the patch locally
> > and re-testing the affected testcase.
>
> I can reproduce that too, seems to be the
> @@ -6896,8 +6897,8 @@
>      }
>    /* Handle zero as null pointer warnings for cases other
>       than EQ_EXPR and NE_EXPR */
> -  else if (null_ptr_cst_p (expr) &&
> -          (TYPE_PTR_OR_PTRMEM_P (totype) || NULLPTR_TYPE_P (totype)))
> +  else if ((TYPE_PTR_OR_PTRMEM_P (totype) || NULLPTR_TYPE_P (totype))
> +          && null_ptr_cst_p (expr))
>      {
>        location_t loc = get_location_for_expr_unwinding_for_system_header (expr);
>        maybe_warn_zero_as_null_pointer_constant (expr, loc);
> hunk.  The Wunused-var-35.C patch hasn't been posted to gcc-patches, but
> judging from the corresponding PR, I'd say the right thing is below.
>
> Regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2019-04-25  Jakub Jelinek  <jakub@redhat.com>
>
>         PR c++/44648
>         * g++.dg/warn/Wunused-var-35.C: Remove xfail.
>
> --- gcc/testsuite/g++.dg/warn/Wunused-var-35.C.jj       2019-02-04 09:44:31.365413407 +0100
> +++ gcc/testsuite/g++.dg/warn/Wunused-var-35.C  2019-04-25 13:24:49.717065815 +0200
> @@ -11,9 +11,8 @@ int main()
>    else
>      return 1;
>
> -  if (const bool b2 = 1) // { dg-warning "\\\[-Wunused-variable\\\]" "bug" { xfail c++98_only } }
> +  if (const bool b2 = 1) // { dg-warning "\\\[-Wunused-variable\\\]" }
>      return 0;
>    else
>      return 1;
>  }
> -
>
>
>         Jakub