[C++] Don't fold __builtin_constant_p prematurely

Message ID alpine.DEB.2.02.1907101722500.29179@grove.saclay.inria.fr
State New
Headers show
Series
  • [C++] Don't fold __builtin_constant_p prematurely
Related show

Commit Message

Marc Glisse July 10, 2019, 3:32 p.m.
Hello,

this avoids folding __builtin_constant_p to 0 early when we are not forced 
to do so. Clearly this has an effect, since it uncovered a bug in 
wi::lshift, fixed today ;-)

I wasn't sure about using |= or just =, the first one seemed more 
conservative.

Bootstrap+regtest on x86_64-pc-linux-gnu.

2019-07-11  Marc Glisse  <marc.glisse@inria.fr>

gcc/cp/
 	* constexpr.c (cxx_eval_builtin_function_call): Only set
 	force_folding_builtin_constant_p if manifestly_const_eval.

gcc/testsuite/
 	* g++.dg/pr85746.C: New file.

-- 
Marc Glisse

Comments

Marc Glisse July 16, 2019, 9:24 p.m. | #1
Adding a C++ maintainer in Cc:
https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00808.html

On Wed, 10 Jul 2019, Marc Glisse wrote:

> Hello,

>

> this avoids folding __builtin_constant_p to 0 early when we are not forced to 

> do so. Clearly this has an effect, since it uncovered a bug in wi::lshift, 

> fixed today ;-)

>

> I wasn't sure about using |= or just =, the first one seemed more 

> conservative.

>

> Bootstrap+regtest on x86_64-pc-linux-gnu.

>

> 2019-07-11  Marc Glisse  <marc.glisse@inria.fr>

>

> gcc/cp/

> 	* constexpr.c (cxx_eval_builtin_function_call): Only set

> 	force_folding_builtin_constant_p if manifestly_const_eval.

>

> gcc/testsuite/

> 	* g++.dg/pr85746.C: New file.


-- 
Marc Glisse
Marc Glisse Aug. 2, 2019, 12:11 p.m. | #2
Ping

On Tue, 16 Jul 2019, Marc Glisse wrote:

> Adding a C++ maintainer in Cc:

> https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00808.html

>

> On Wed, 10 Jul 2019, Marc Glisse wrote:

>

>> Hello,

>> 

>> this avoids folding __builtin_constant_p to 0 early when we are not forced 

>> to do so. Clearly this has an effect, since it uncovered a bug in 

>> wi::lshift, fixed today ;-)

>> 

>> I wasn't sure about using |= or just =, the first one seemed more 

>> conservative.

>> 

>> Bootstrap+regtest on x86_64-pc-linux-gnu.

>> 

>> 2019-07-11  Marc Glisse  <marc.glisse@inria.fr>

>> 

>> gcc/cp/

>> 	* constexpr.c (cxx_eval_builtin_function_call): Only set

>> 	force_folding_builtin_constant_p if manifestly_const_eval.

>> 

>> gcc/testsuite/

>> 	* g++.dg/pr85746.C: New file.


-- 
Marc Glisse
Marc Glisse Sept. 3, 2019, 6:38 a.m. | #3
On Fri, 2 Aug 2019, Marc Glisse wrote:

> Ping

>

> On Tue, 16 Jul 2019, Marc Glisse wrote:

>

>> Adding a C++ maintainer in Cc:

>> https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00808.html

>> 

>> On Wed, 10 Jul 2019, Marc Glisse wrote:

>> 

>>> Hello,

>>> 

>>> this avoids folding __builtin_constant_p to 0 early when we are not forced 

>>> to do so. Clearly this has an effect, since it uncovered a bug in 

>>> wi::lshift, fixed today ;-)

>>> 

>>> I wasn't sure about using |= or just =, the first one seemed more 

>>> conservative.

>>> 

>>> Bootstrap+regtest on x86_64-pc-linux-gnu.

>>> 

>>> 2019-07-11  Marc Glisse  <marc.glisse@inria.fr>

>>> 

>>> gcc/cp/

>>> 	* constexpr.c (cxx_eval_builtin_function_call): Only set

>>> 	force_folding_builtin_constant_p if manifestly_const_eval.

>>> 

>>> gcc/testsuite/

>>> 	* g++.dg/pr85746.C: New file.


-- 
Marc Glisse
Marc Glisse Sept. 11, 2019, 3:49 p.m. | #4
Ping

On Tue, 3 Sep 2019, Marc Glisse wrote:

> On Fri, 2 Aug 2019, Marc Glisse wrote:

>

>> Ping

>> 

>> On Tue, 16 Jul 2019, Marc Glisse wrote:

>> 

>>> Adding a C++ maintainer in Cc:

>>> https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00808.html

>>> 

>>> On Wed, 10 Jul 2019, Marc Glisse wrote:

>>> 

>>>> Hello,

>>>> 

>>>> this avoids folding __builtin_constant_p to 0 early when we are not 

>>>> forced to do so. Clearly this has an effect, since it uncovered a bug in 

>>>> wi::lshift, fixed today ;-)

>>>> 

>>>> I wasn't sure about using |= or just =, the first one seemed more 

>>>> conservative.

>>>> 

>>>> Bootstrap+regtest on x86_64-pc-linux-gnu.

>>>> 

>>>> 2019-07-11  Marc Glisse  <marc.glisse@inria.fr>

>>>> 

>>>> gcc/cp/

>>>> 	* constexpr.c (cxx_eval_builtin_function_call): Only set

>>>> 	force_folding_builtin_constant_p if manifestly_const_eval.

>>>> 

>>>> gcc/testsuite/

>>>> 	* g++.dg/pr85746.C: New file.


-- 
Marc Glisse
Marc Glisse Sept. 27, 2019, 6:07 p.m. | #5
Ping https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00808.html
with one more potential reviewer in Cc.

On Wed, 11 Sep 2019, Marc Glisse wrote:

> Ping

>

> On Tue, 3 Sep 2019, Marc Glisse wrote:

>

>> On Fri, 2 Aug 2019, Marc Glisse wrote:

>> 

>>> Ping

>>> 

>>> On Tue, 16 Jul 2019, Marc Glisse wrote:

>>> 

>>>> Adding a C++ maintainer in Cc:

>>>> https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00808.html

>>>> 

>>>> On Wed, 10 Jul 2019, Marc Glisse wrote:

>>>> 

>>>>> Hello,

>>>>> 

>>>>> this avoids folding __builtin_constant_p to 0 early when we are not 

>>>>> forced to do so. Clearly this has an effect, since it uncovered a bug in 

>>>>> wi::lshift, fixed today ;-)

>>>>> 

>>>>> I wasn't sure about using |= or just =, the first one seemed more 

>>>>> conservative.

>>>>> 

>>>>> Bootstrap+regtest on x86_64-pc-linux-gnu.

>>>>> 

>>>>> 2019-07-11  Marc Glisse  <marc.glisse@inria.fr>

>>>>> 

>>>>> gcc/cp/

>>>>> 	* constexpr.c (cxx_eval_builtin_function_call): Only set

>>>>> 	force_folding_builtin_constant_p if manifestly_const_eval.

>>>>> 

>>>>> gcc/testsuite/

>>>>> 	* g++.dg/pr85746.C: New file.

>

>


-- 
Marc Glisse
Jason Merrill Oct. 21, 2019, 6:39 p.m. | #6
OK, thanks.

On 9/27/19 2:07 PM, Marc Glisse wrote:
> Ping https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00808.html

> with one more potential reviewer in Cc.

> 

> On Wed, 11 Sep 2019, Marc Glisse wrote:

> 

>> Ping

>>

>> On Tue, 3 Sep 2019, Marc Glisse wrote:

>>

>>> On Fri, 2 Aug 2019, Marc Glisse wrote:

>>>

>>>> Ping

>>>>

>>>> On Tue, 16 Jul 2019, Marc Glisse wrote:

>>>>

>>>>> Adding a C++ maintainer in Cc:

>>>>> https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00808.html

>>>>>

>>>>> On Wed, 10 Jul 2019, Marc Glisse wrote:

>>>>>

>>>>>> Hello,

>>>>>>

>>>>>> this avoids folding __builtin_constant_p to 0 early when we are 

>>>>>> not forced to do so. Clearly this has an effect, since it 

>>>>>> uncovered a bug in wi::lshift, fixed today ;-)

>>>>>>

>>>>>> I wasn't sure about using |= or just =, the first one seemed more 

>>>>>> conservative.

>>>>>>

>>>>>> Bootstrap+regtest on x86_64-pc-linux-gnu.

>>>>>>

>>>>>> 2019-07-11  Marc Glisse  <marc.glisse@inria.fr>

>>>>>>

>>>>>> gcc/cp/

>>>>>>     * constexpr.c (cxx_eval_builtin_function_call): Only set

>>>>>>     force_folding_builtin_constant_p if manifestly_const_eval.

>>>>>>

>>>>>> gcc/testsuite/

>>>>>>     * g++.dg/pr85746.C: New file.

>>

>>

>

Patch

Index: gcc/cp/constexpr.c
===================================================================
--- gcc/cp/constexpr.c	(revision 273356)
+++ gcc/cp/constexpr.c	(working copy)
@@ -1248,21 +1248,21 @@  cxx_eval_builtin_function_call (const co
 						  &dummy1, &dummy2);
 	}
 
       if (bi_const_p)
 	/* For __builtin_constant_p, fold all expressions with constant values
 	   even if they aren't C++ constant-expressions.  */
 	args[i] = cp_fold_rvalue (args[i]);
     }
 
   bool save_ffbcp = force_folding_builtin_constant_p;
-  force_folding_builtin_constant_p = true;
+  force_folding_builtin_constant_p |= ctx->manifestly_const_eval;
   tree save_cur_fn = current_function_decl;
   /* Return name of ctx->call->fundef->decl for __builtin_FUNCTION ().  */
   if (fndecl_built_in_p (fun, BUILT_IN_FUNCTION)
       && ctx->call
       && ctx->call->fundef)
     current_function_decl = ctx->call->fundef->decl;
   new_call = fold_builtin_call_array (EXPR_LOCATION (t), TREE_TYPE (t),
 				      CALL_EXPR_FN (t), nargs, args);
   current_function_decl = save_cur_fn;
   force_folding_builtin_constant_p = save_ffbcp;
Index: gcc/testsuite/g++.dg/pr85746.C
===================================================================
--- gcc/testsuite/g++.dg/pr85746.C	(nonexistent)
+++ gcc/testsuite/g++.dg/pr85746.C	(working copy)
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-gimple" } */
+
+int f(int a,int b){
+  // The front-end should not fold this to 0.
+  int c = __builtin_constant_p(a < b);
+  return c;
+}
+
+/* { dg-final { scan-tree-dump "__builtin_constant_p" "gimple" } } */