Re: [PATCH v8] c++: implement P2564, consteval needs to propagate up [PR107687]
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]
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]
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]
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]
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:
>
>
[PATCH v8] c++: implement P2564, consteval needs to propagate up [PR107687]
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!
-- >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:
int g(int i)
{
f(i); // f is immediate-escalating
}
where g itself is not immediate-escalating, but we have to make sure
that if f gets promoted to consteval, we give an error.
A new option, -fno-immediate-escalation, is provided to suppress
escalating functions.
v2 also adds a new flag, DECL_ESCALATION_CHECKED_P, so that we don't
escalate a function multiple times, and so that we can distinguish between
explicitly consteval functions and functions that have been promoted
to consteval.
In v3, I removed one of the new vectors and changed the other one
to a hash set. This version also contains numerous cleanups.
v4 merges find_escalating_expr_r into cp_fold_immediate_r. It also
adds a new optimization in cp_fold_function.
v5 greatly simplifies the code.
v6 simplifies the code further and removes an ff_ flag.
v7 removes maybe_promote_function_to_consteval and further simplifies
cp_fold_immediate_r logic.
v8 removes maybe_store_immediate_escalating_fn.
PR c++/107687
PR c++/110997
gcc/c-family/ChangeLog:
* c-cppbuiltin.cc (c_cpp_built
