Re: [PATCH v8] c++: implement P2564, consteval needs to propagate up [PR107687]

2023-12-11 Thread Marek Polacek
On Mon, Dec 11, 2023 at 02:02:52PM -0500, Jason Merrill wrote:
> On 12/11/23 03:49, FX Coudert wrote:
> > Hi Marek,
> > 
> > The patch is causing three failures on x86_64-apple-darwin21:
> > 
> > > FAIL: g++.dg/cpp2a/concepts-explicit-inst1.C -std=c++20 scan-assembler 
> > > _Z1gI1XEvT_
> > > FAIL: g++.dg/cpp2a/concepts-explicit-inst1.C -std=c++20 scan-assembler 
> > > _Z1gI1YEvT_
> 
> I think these are from my r14-6064-gc3f281a0c1ca50 rather than Marek's
> patch.  The mangling of those two functions changed, but the test looking
> for the old mangling still passed on targets that support ABI compatibility
> aliases.  I'll fix the tests.

Sounds plausible, thanks.  If the tests still fail with 
-fno-immediate-escalation,
it's the mangling changes.
 
> Jason
> 
> > > FAIL: g++.dg/cpp2a/consteval-prop6.C -std=c++20 at line 58 (test for 
> > > warnings, line 57)
> > 
> > How could I help debug this?
> > 
> > FX
> > 
> 

Marek



Re: [PATCH v8] c++: implement P2564, consteval needs to propagate up [PR107687]

2023-12-11 Thread Jason Merrill

On 12/11/23 03:49, FX Coudert wrote:

Hi Marek,

The patch is causing three failures on x86_64-apple-darwin21:


FAIL: g++.dg/cpp2a/concepts-explicit-inst1.C -std=c++20 scan-assembler 
_Z1gI1XEvT_
FAIL: g++.dg/cpp2a/concepts-explicit-inst1.C -std=c++20 scan-assembler 
_Z1gI1YEvT_


I think these are from my r14-6064-gc3f281a0c1ca50 rather than Marek's 
patch.  The mangling of those two functions changed, but the test 
looking for the old mangling still passed on targets that support ABI 
compatibility aliases.  I'll fix the tests.


Jason


FAIL: g++.dg/cpp2a/consteval-prop6.C -std=c++20 at line 58 (test for warnings, 
line 57)


How could I help debug this?

FX





Re: [PATCH v8] c++: implement P2564, consteval needs to propagate up [PR107687]

2023-12-11 Thread FX Coudert
Hi Marek,

The patch is causing three failures on x86_64-apple-darwin21:

> FAIL: g++.dg/cpp2a/concepts-explicit-inst1.C -std=c++20 scan-assembler 
> _Z1gI1XEvT_
> FAIL: g++.dg/cpp2a/concepts-explicit-inst1.C -std=c++20 scan-assembler 
> _Z1gI1YEvT_
> FAIL: g++.dg/cpp2a/consteval-prop6.C -std=c++20 at line 58 (test for 
> warnings, line 57)

How could I help debug this?

FX

Re: [PATCH v8] c++: implement P2564, consteval needs to propagate up [PR107687]

2023-12-06 Thread Marek Polacek
On Wed, Dec 06, 2023 at 05:09:21PM +0530, Prathamesh Kulkarni wrote:
> On Tue, 5 Dec 2023 at 06:18, Marek Polacek  wrote:
> >
> > On Mon, Dec 04, 2023 at 04:49:29PM -0500, Jason Merrill wrote:
> > > On 12/4/23 15:23, Marek Polacek wrote:
> > > > +/* FN is not a consteval function, but may become one.  Remember to
> > > > +   escalate it after all pending templates have been instantiated.  */
> > > > +
> > > > +void
> > > > +maybe_store_immediate_escalating_fn (tree fn)
> > > > +{
> > > > +  if (unchecked_immediate_escalating_function_p (fn))
> > > > +remember_escalating_expr (fn);
> > > > +}
> > >
> > > > +++ b/gcc/cp/decl.cc
> > > > @@ -18441,7 +18441,10 @@ finish_function (bool inline_p)
> > > >if (!processing_template_decl
> > > >&& !DECL_IMMEDIATE_FUNCTION_P (fndecl)
> > > >&& !DECL_OMP_DECLARE_REDUCTION_P (fndecl))
> > > > -cp_fold_function (fndecl);
> > > > +{
> > > > +  cp_fold_function (fndecl);
> > > > +  maybe_store_immediate_escalating_fn (fndecl);
> > > > +}
> > >
> > > I think maybe_store_, and the call to it from finish_function, are 
> > > unneeded;
> > > we will have already decided whether we need to remember the function 
> > > during
> > > the call to cp_fold_function.
> >
> > 'Tis true.
> >
> > > OK with that change.
> >
> > Here's what I pushed after another regtest.  Thanks!
> Hi Marek,
> It seems the patch caused following regressions on aarch64:
> 
> Running g++:g++.dg/modules/modules.exp ...
> FAIL: g++.dg/modules/xtreme-header-4_b.C -std=c++2b (internal compiler
> error: tree check: expected class 'type', have 'declaration'
> (template_decl) in get_originating_module_decl, at cp/module.cc:18659)
> FAIL: g++.dg/modules/xtreme-header-5_b.C -std=c++2b (internal compiler
> error: tree check: expected class 'type', have 'declaration'
> (template_decl) in get_originating_module_decl, at cp/module.cc:18659)
> FAIL: g++.dg/modules/xtreme-header_b.C -std=c++2b (internal compiler
> error: tree check: expected class 'type', have 'declaration'
> (template_decl) in get_originating_module_decl, at cp/module.cc:18659)
> 
> Log files: 
> https://ci.linaro.org/job/tcwg_gcc_check--master-aarch64-build/1299/artifact/artifacts/00-sumfiles/

Are you sure it's caused by my patch?  I reckon I've seen that FAIL
many times before.

Marek



Re: [PATCH v8] c++: implement P2564, consteval needs to propagate up [PR107687]

2023-12-06 Thread Prathamesh Kulkarni
On Tue, 5 Dec 2023 at 06:18, Marek Polacek  wrote:
>
> On Mon, Dec 04, 2023 at 04:49:29PM -0500, Jason Merrill wrote:
> > On 12/4/23 15:23, Marek Polacek wrote:
> > > +/* FN is not a consteval function, but may become one.  Remember to
> > > +   escalate it after all pending templates have been instantiated.  */
> > > +
> > > +void
> > > +maybe_store_immediate_escalating_fn (tree fn)
> > > +{
> > > +  if (unchecked_immediate_escalating_function_p (fn))
> > > +remember_escalating_expr (fn);
> > > +}
> >
> > > +++ b/gcc/cp/decl.cc
> > > @@ -18441,7 +18441,10 @@ finish_function (bool inline_p)
> > >if (!processing_template_decl
> > >&& !DECL_IMMEDIATE_FUNCTION_P (fndecl)
> > >&& !DECL_OMP_DECLARE_REDUCTION_P (fndecl))
> > > -cp_fold_function (fndecl);
> > > +{
> > > +  cp_fold_function (fndecl);
> > > +  maybe_store_immediate_escalating_fn (fndecl);
> > > +}
> >
> > I think maybe_store_, and the call to it from finish_function, are unneeded;
> > we will have already decided whether we need to remember the function during
> > the call to cp_fold_function.
>
> 'Tis true.
>
> > OK with that change.
>
> Here's what I pushed after another regtest.  Thanks!
Hi Marek,
It seems the patch caused following regressions on aarch64:

Running g++:g++.dg/modules/modules.exp ...
FAIL: g++.dg/modules/xtreme-header-4_b.C -std=c++2b (internal compiler
error: tree check: expected class 'type', have 'declaration'
(template_decl) in get_originating_module_decl, at cp/module.cc:18659)
FAIL: g++.dg/modules/xtreme-header-5_b.C -std=c++2b (internal compiler
error: tree check: expected class 'type', have 'declaration'
(template_decl) in get_originating_module_decl, at cp/module.cc:18659)
FAIL: g++.dg/modules/xtreme-header_b.C -std=c++2b (internal compiler
error: tree check: expected class 'type', have 'declaration'
(template_decl) in get_originating_module_decl, at cp/module.cc:18659)

Log files: 
https://ci.linaro.org/job/tcwg_gcc_check--master-aarch64-build/1299/artifact/artifacts/00-sumfiles/

Thanks,
Prathamesh
>
> -- >8 --
> This patch implements P2564, described at , whereby
> certain functions are promoted to consteval.  For example:
>
>   consteval int id(int i) { return i; }
>
>   template 
>   constexpr int f(T t)
>   {
> return t + id(t); // id causes f to be promoted to consteval
>   }
>
>   void g(int i)
>   {
> f (3);
>   }
>
> now compiles.  Previously the code was ill-formed: we would complain
> that 't' in 'f' is not a constant expression.  Since 'f' is now
> consteval, it means that the call to id(t) is in an immediate context,
> so doesn't have to produce a constant -- this is how we allow consteval
> functions composition.  But making 'f' consteval also means that
> the call to 'f' in 'g' must yield a constant; failure to do so results
> in an error.  I made the effort to have cc1plus explain to us what's
> going on.  For example, calling f(i) produces this neat diagnostic:
>
> w.C:11:11: error: call to consteval function 'f(i)' is not a constant 
> expression
>11 | f (i);
>   | ~~^~~
> w.C:11:11: error: 'i' is not a constant expression
> w.C:6:22: note: 'constexpr int f(T) [with T = int]' was promoted to an 
> immediate function because its body contains an immediate-escalating 
> expression 'id(t)'
> 6 | return t + id(t); // id causes f to be promoted to 
> consteval
>   |~~^~~
>
> which hopefully makes it clear what's going on.
>
> Implementing this proposal has been tricky.  One problem was delayed
> instantiation: instantiating a function can set off a domino effect
> where one call promotes a function to consteval but that then means
> that another function should also be promoted, etc.
>
> In v1, I addressed the delayed instantiation problem by instantiating
> trees early, so that we can escalate functions right away.  That caused
> a number of problems, and in certain cases, like consteval-prop3.C, it
> can't work, because we need to wait till EOF to see the definition of
> the function anyway.  Overeager instantiation tends to cause diagnostic
> problems too.
>
> In v2, I attempted to move the escalation to the gimplifier, at which
> point all templates have been instantiated.  That attempt flopped,
> however, because once we've gimplified a function, its body is discarded
> and as a consequence, you can no longer evaluate a call to that function
> which is required for escalating, which needs to decide if a call is
> a constant expression or not.
>
> Therefore, we have to perform the escalation before gimplifying, but
> after instantiate_pending_templates.  That's not easy because we have
> no way to walk all the trees.  In the v2 patch, I use two vectors: one
> to store function decls that may become consteval, and another to
> remember references to immediate-escalating functions.  Unfortunately
> the latter must also stash functions that call immediate-escalating
> functions.  Consider:
>
>