[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243 --- Comment #13 from CVS Commits --- The master branch has been updated by Patrick Palka : https://gcc.gnu.org/g:5425159d176a7a92afc932cbb22d8822667099c4 commit r13-6422-g5425159d176a7a92afc932cbb22d8822667099c4 Author: Patrick Palka Date: Thu Mar 2 14:04:50 2023 -0500 c++: more mce_false folding from cp_fully_fold_init [PR108243] We should also fold the overall initializer passed to cp_fully_fold_init with mce_false, which allows folding of the copy-initialization of 'a1' in the below testcase (the initializer here is an AGGR_INIT_EXPR). Unfortunately this doesn't help with direct- or default-initialization because we don't call cp_fully_fold_init in that case, and even if we did the initializer in that case is expressed as a bare CALL_EXPR instead of an AGGR_INIT_EXPR, which cp_fully_fold_init can't really fold. PR c++/108243 PR c++/97553 gcc/cp/ChangeLog: * cp-gimplify.cc (cp_fully_fold): Add an internal overload that additionally takes and propagate an mce_value parameter, and define the existing public overload in terms of it. (cp_fully_fold_init): Pass mce_false to cp_fully_fold. gcc/testsuite/ChangeLog: * g++.dg/opt/is_constant_evaluated3.C: New test.
[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243 --- Comment #12 from CVS Commits --- The master branch has been updated by Patrick Palka : https://gcc.gnu.org/g:cbaa1d9c218d9c0b5e34e510a462ff4e299a0f3f commit r13-6421-gcbaa1d9c218d9c0b5e34e510a462ff4e299a0f3f Author: Patrick Palka Date: Thu Mar 2 14:03:21 2023 -0500 c++: constant non-copy-init is manifestly constant [PR108243] According to [basic.start.static]/2 and [expr.const]/2, a variable with static storage duration initialized with a constant initializer has constant initialization, and such an initializer is manifestly constant-evaluated. For copy initialization, we're already getting this right because in that case check_initializer would consistently call store_init_value, which for TREE_STATIC variables calls fold_non_dependent_init with m_c_e=true. But for direct (or default) initialization, check_initializer doesn't always call store_init_value. We instead however always call maybe_constant_init from expand_default_init[1], albeit with m_c_e=false which means we don't get the "manifestly constant-evaluated" part right for non-copy-init. This patch fixes this by setting m_c_e=true in maybe_constant_init for static storage duration variables, mainly for benefit of the call to maybe_constant_init from expand_default_init. [1]: this maybe_constant_init call isn't reached in the copy-init case because there init is a CONSTRUCTOR rather than a TREE_LIST, and so we exit early from expand_default_init, returning an INIT_EXPR. This INIT_EXPR is ultimately what causes us to consistently hit the store_init_value code path from check_initializer in the copy-init case. PR c++/108243 gcc/cp/ChangeLog: * constexpr.cc (maybe_constant_init_1): Override manifestly_const_eval to true if is_static. gcc/testsuite/ChangeLog: * g++.dg/cpp2a/is-constant-evaluated14.C: New test.
[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243
--- Comment #11 from Patrick Palka ---
The pending patch
https://gcc.gnu.org/pipermail/gcc-patches/2023-February/612365.html will fix
that. With that patch we'll statically initialize
static const string_view foo("bar");
as we already do for
static const string_view foo = {"bar"};
[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243
Richard Biener changed:
What|Removed |Added
Priority|P3 |P2
--- Comment #10 from Richard Biener ---
I'm not sure if this is supposed to be fixed yet. For the comment#2 testcase
I now get
[local count: 1073741824]:
_1 = __atomic_load_1 (&_ZGVZ4mainE3foo, 2);
if (_1 == 0)
goto ; [33.00%]
else
goto ; [67.00%]
[local count: 956811341]:
goto ; [100.00%]
[local count: 354334800]:
_2 = __cxa_guard_acquire (&_ZGVZ4mainE3foo);
if (_2 != 0)
goto ; [33.00%]
else
goto ; [67.00%]
[local count: 116930483]:
MEM[(struct string_view *)] ={v} {CLOBBER};
MEM[(struct string_view *)].str = "bar";
MEM[(struct string_view *)].len = 3;
__cxa_guard_release (&_ZGVZ4mainE3foo);
[local count: 1073741824]:
_9 = foo.len;
_8 = (int) _9;
return _8;
so we fail to CSE foo.len because __cxa_guard_release possibly clobbers it
(it's global memory).
Note the clobbering is necessary to prevent code motion across the
lock primitives.
Note that __cxa_guard_release also prevents removing the .str initialization
since only the .len member is ever inspected. As noted static initialization
would improve this quite a bit.
[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243
--- Comment #9 from Patrick Palka ---
(In reply to Jiang An from comment #8)
> This seems a bug, not merely missing of optimization. The string_view object
> should always be statically initialized, because it has static storage
> duration and its initializer is a constant iniitializer.
Good point. It looks like we're inconsistent about static initialization here
-- we do it for suitable copy initialization but not for suitable direct
initialization:
static std::string_view foo1 = {"bar"}; // static initialization
static std::string_view foo2("bar");// dynamic initialization
[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243 Jiang An changed: What|Removed |Added CC||de34 at live dot cn --- Comment #8 from Jiang An --- This seems a bug, not merely missing of optimization. The string_view object should always be statically initialized, because it has static storage duration and its initializer is a constant iniitializer.
[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243 --- Comment #7 from CVS Commits --- The master branch has been updated by Patrick Palka : https://gcc.gnu.org/g:5fea1be820508e1fbc610d1a54b61c1add33c36f commit r13-6120-g5fea1be820508e1fbc610d1a54b61c1add33c36f Author: Patrick Palka Date: Fri Feb 17 15:18:10 2023 -0500 c++: speculative constexpr and is_constant_evaluated [PR108243] This PR illustrates that __builtin_is_constant_evaluated currently acts as an optimization barrier for our speculative constexpr evaluation, since we don't want to prematurely fold the builtin to false before the expression in question undergoes manifestly constant evaluation if appropriate (in which case the builtin must instead be folded to true). This patch fixes this by permitting __builtin_is_constant_evaluated to get folded as false at appropiate points, namely during cp_fold_function and cp_fully_fold_init where we know we're done with manifestly constant evaluation. The function cp_fold gets a flags parameter that controls whether we pass mce_false or mce_unknown to maybe_constant_value when folding a CALL_EXPR. PR c++/108243 PR c++/97553 gcc/cp/ChangeLog: * cp-gimplify.cc (enum fold_flags): Define. (fold_flags_t): Declare. (cp_fold_data::genericize): Replace this data member with ... (cp_fold_data::fold_flags): ... this. (cp_fold_r): Adjust use of cp_fold_data and calls to cp_fold. (cp_fold_function): Likewise. (cp_fold_maybe_rvalue): Add an internal overload that additionally takes and propagates a fold_flags_t parameter, and define the existing public overload in terms of it. (cp_fold_rvalue): Likewise. (cp_fully_fold_init): Adjust use of cp_fold_data. (fold_cache): Replace with ... (fold_caches): ... this 2-element array of caches. (get_fold_cache): Define. (clear_fold_cache): Adjust. (cp_fold): Add fold_flags_t parameter. Use get_fold_cache. Pass flags to calls to cp_fold, cp_fold_rvalue and cp_fold_maybe_rvalue. : If ff_mce_false is set, fold __builtin_is_constant_evaluated to false and pass mce_false to maybe_constant_value. gcc/testsuite/ChangeLog: * g++.dg/opt/is_constant_evaluated1.C: New test. * g++.dg/opt/is_constant_evaluated2.C: New test.
[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243 Patrick Palka changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |ppalka at gcc dot gnu.org Status|NEW |ASSIGNED
[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243 Richard Biener changed: What|Removed |Added Target Milestone|--- |10.5
[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #6 from Jakub Jelinek --- We could have some FE constant expression evaluation mode in which we'd guarantee that if the initializer/expression was manifestly constant evaluated, it has been constant expression evaluated with manifestly_const_eval=true already and do that as a last resort folding of initializers or perhaps expressions in cp_fold_function or so. But as you wrote, we can't do that by default for !manifestly_const_eval evaluation, because we often do that before we evaluate those with manifestly_const_eval=true.
[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243 Patrick Palka changed: What|Removed |Added See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=97553 CC||ppalka at gcc dot gnu.org --- Comment #5 from Patrick Palka --- IIUC __builtin_is_constant_evaluated() currently acts as an optimization barrier for the frontend's speculative folding since we don't want to prematurely fold is_constant_evaluated() to false if the expression in question would later be manifestly constant evaluated (in which case is_constant_evaluated() must be folded to true). PR97553 is caused by the same issue I think.
[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243
--- Comment #4 from Jonathan Wakely ---
GCC can't even optimize it properly for C++20 when using if-consteval:
static constexpr unsigned long length(const char* s)
{
#ifndef FIX
if consteval {
unsigned long n = 0;
while (*s++)
++n;
return n;
}
#endif
return __builtin_strlen(s);
}
[Bug c++/108243] [10/11/12/13 Regression] Missed optimization for static const std::string_view(const char*)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108243
Jonathan Wakely changed:
What|Removed |Added
Summary|Missed optimization for |[10/11/12/13 Regression]
|static const|Missed optimization for
|std::string_view(const |static const
|char*) |std::string_view(const
||char*)
--- Comment #3 from Jonathan Wakely ---
And if the variable is like this it does optimize it:
static constexpr string_view foo("bar");
Because then we take the constant evaluation path and calculate the length by
hand.
The problem seems to be that GCC considers the
__builtin_is_constant_evaluated() branch to be reachable for non-constexpr and
so doesn't inline as aggressively. If we have:
struct traits
{
static constexpr unsigned long length(const char* s)
{
#ifndef FIX
if (__builtin_is_constant_evaluated())
{
unsigned long n = 0;
while (*s++)
++n;
return n;
}
#endif
return __builtin_strlen(s);
}
};
struct string_view
{
constexpr
string_view(const char* s) : str(s), len(traits::length(s)) { }
unsigned long size() const { return len; }
const char* str;
unsigned long len;
};
int main()
{
static const string_view foo("bar");
return foo.size();
}
Then with -DFIX we optimize correctly, but without -DFIX we don't. That block
of code should make no difference to inlining decisions, because at runtime it
is always dead code.
