[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 Richard Biener changed: What|Removed |Added Resolution|--- |INVALID Status|UNCONFIRMED |RESOLVED --- Comment #16 from Richard Biener --- So it's not invalid to elide this (for C++) but instead conflicting specification by the user.
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 Bug 100409 depends on bug 101087, which changed state. Bug 101087 Summary: [9 Regression] Unevaluated operand of sizeof affects noexcept operator https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101087 What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 --- Comment #15 from Jason Merrill --- (In reply to Richard Biener from comment #14) > Does that mean C++ should default to -fdelete-dead-exceptions? That makes sense. The C++ standard has nothing to say about this, since pure/const are extensions. And throwing an exception from a signal handler is undefined behavior.
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 --- Comment #14 from Richard Biener --- (In reply to Jason Merrill from comment #13) > (In reply to Richard Biener from comment #3) > > - if (! TREE_SIDE_EFFECTS (expr)) > > + if (! TREE_SIDE_EFFECTS (expr) && expr_noexcept_p (expr, 0)) > > expr = void_node; > > The assumption that an expression with TREE_SIDE_EFFECTS unset can be > discarded if its value is not used is made throughout the compiler. > Abandoning that invariant seems like a bad idea. And changing from checking > a flag to walking through the whole expression in all of those places sounds > like an algorithmic complexity problem. > > If a pure/const function can throw, fine. But that shouldn't affect how > optimization treats the function call, or we'll end up pessimizing the vast > majority of calls to pure/const functions that don't throw (but are not > explicitly declared noexcept). In this testcase, if the user doesn't want > the compiler to optimize away a call to foo(), they shouldn't mark it pure. Hmm, fair enough. Does that mean C++ should default to -fdelete-dead-exceptions? Elsewhere it's said that TREE_SIDE_EFFECTS, const/pure and EH are orthogonal concepts but yes, that we have to walk expressions recursively to figure nothrow sucks. Though the convert_to_void "optimization" could be seen as premature, on GIMPLE there's no recursive walking needed and nothrow discovery should discover nothrow-ness of most small functions that have the body available - so the other option I considered was to remove that optimization. Now, this C++ decision makes my life a bit harder (writing testcases for the middle-end issues around throwing pure/const functions).
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 --- Comment #13 from Jason Merrill --- (In reply to Richard Biener from comment #3) > - if (! TREE_SIDE_EFFECTS (expr)) > + if (! TREE_SIDE_EFFECTS (expr) && expr_noexcept_p (expr, 0)) > expr = void_node; The assumption that an expression with TREE_SIDE_EFFECTS unset can be discarded if its value is not used is made throughout the compiler. Abandoning that invariant seems like a bad idea. And changing from checking a flag to walking through the whole expression in all of those places sounds like an algorithmic complexity problem. If a pure/const function can throw, fine. But that shouldn't affect how optimization treats the function call, or we'll end up pessimizing the vast majority of calls to pure/const functions that don't throw (but are not explicitly declared noexcept). In this testcase, if the user doesn't want the compiler to optimize away a call to foo(), they shouldn't mark it pure.
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 Marek Polacek changed: What|Removed |Added CC||mpolacek at gcc dot gnu.org --- Comment #12 from Marek Polacek --- (In reply to Richard Biener from comment #10) > (In reply to Richard Biener from comment #7) > > Jason, any idea? > > diff --git a/gcc/cp/except.c b/gcc/cp/except.c > index a8cea53cf91..cbc94dff790 100644 > --- a/gcc/cp/except.c > +++ b/gcc/cp/except.c > @@ -1050,7 +1050,7 @@ check_noexcept_r (tree *tp, int * /*walk_subtrees*/, > void * /*data*/) >if (concept_check_p (fn)) > return NULL_TREE; >tree type = TREE_TYPE (fn); > - gcc_assert (INDIRECT_TYPE_P (type)); > + gcc_assert (INDIRECT_TYPE_P (type) || type == unknown_type_node); >type = TREE_TYPE (type); > >STRIP_NOPS (fn); > @@ -1073,7 +1073,7 @@ check_noexcept_r (tree *tp, int * /*walk_subtrees*/, > void * /*data*/) > to evaluate the noexcept-specifier's operand here, but that > could > cause instantiations that would fail. */ > } > - if (!TYPE_NOTHROW_P (type)) > + if (type == unknown_type_node || !TYPE_NOTHROW_P (type)) > return fn; > } > > > fixes all but the ICE for g++.dg/warn/noeffect4.C where we end up with > an invalid CALL_EXPR: > > arg:0 long unsigned int> > readonly > arg:0 0x765615e8 int> > side-effects tree_6 > fn 0x766a5f18> > > > also notice how sizeof() doesn't have side-effects but the call has. I > suppose that's somehow an "unevaluated" sizeof() expression we don't > expect to hit expr_noexcept_p? OTOH noexcept(sizeof(foo())) might be > "valid"? Incidentally I just posted a patch for noexcept(sizeof(foo())) because we got the result wrong (I hadn't been aware of this PR): bug 101087. Not sure if that is going to help here, but it could.
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409
Richard Biener changed:
What|Removed |Added
Depends on||101087
--- Comment #11 from Richard Biener ---
(In reply to Richard Biener from comment #10)
> (In reply to Richard Biener from comment #7)
> > Jason, any idea?
...
> which leaves us with
>
> FAIL: g++.dg/warn/noeffect4.C -std=gnu++98 (test for warnings, line 84)
> FAIL: g++.dg/warn/noeffect4.C -std=gnu++14 (test for warnings, line 84)
> FAIL: g++.dg/warn/noeffect4.C -std=gnu++17 (test for warnings, line 84)
> FAIL: g++.dg/warn/noeffect4.C -std=gnu++2a (test for warnings, line 84)
>
> not diagnosing
>
> sizeof (x++);// { dg-warning "no effect" }
>
> not sure what should happen to the x++ side-effect (throwing).
Ah, that's likely PR101087
Referenced Bugs:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101087
[Bug 101087] [9/10/11/12 Regression] Unevaluated operand of sizeof affects
noexcept operator
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409
--- Comment #10 from Richard Biener ---
(In reply to Richard Biener from comment #7)
> Jason, any idea?
diff --git a/gcc/cp/except.c b/gcc/cp/except.c
index a8cea53cf91..cbc94dff790 100644
--- a/gcc/cp/except.c
+++ b/gcc/cp/except.c
@@ -1050,7 +1050,7 @@ check_noexcept_r (tree *tp, int * /*walk_subtrees*/, void
* /*data*/)
if (concept_check_p (fn))
return NULL_TREE;
tree type = TREE_TYPE (fn);
- gcc_assert (INDIRECT_TYPE_P (type));
+ gcc_assert (INDIRECT_TYPE_P (type) || type == unknown_type_node);
type = TREE_TYPE (type);
STRIP_NOPS (fn);
@@ -1073,7 +1073,7 @@ check_noexcept_r (tree *tp, int * /*walk_subtrees*/, void
* /*data*/)
to evaluate the noexcept-specifier's operand here, but that could
cause instantiations that would fail. */
}
- if (!TYPE_NOTHROW_P (type))
+ if (type == unknown_type_node || !TYPE_NOTHROW_P (type))
return fn;
}
fixes all but the ICE for g++.dg/warn/noeffect4.C where we end up with
an invalid CALL_EXPR:
arg:0
readonly
arg:0
side-effects tree_6
fn
also notice how sizeof() doesn't have side-effects but the call has. I
suppose that's somehow an "unevaluated" sizeof() expression we don't
expect to hit expr_noexcept_p? OTOH noexcept(sizeof(foo())) might be
"valid"?
Changing the assert to if (INDIRECT_TYPE_P (type)) conditionalizing the
use of 'type' beyond of course works. Aka
diff --git a/gcc/cp/except.c b/gcc/cp/except.c
index a8cea53cf91..623b83b8dc2 100644
--- a/gcc/cp/except.c
+++ b/gcc/cp/except.c
@@ -1050,8 +1050,6 @@ check_noexcept_r (tree *tp, int * /*walk_subtrees*/, void
* /*data*/)
if (concept_check_p (fn))
return NULL_TREE;
tree type = TREE_TYPE (fn);
- gcc_assert (INDIRECT_TYPE_P (type));
- type = TREE_TYPE (type);
STRIP_NOPS (fn);
if (TREE_CODE (fn) == ADDR_EXPR)
@@ -1073,7 +1071,8 @@ check_noexcept_r (tree *tp, int * /*walk_subtrees*/, void
* /*data*/)
to evaluate the noexcept-specifier's operand here, but that could
cause instantiations that would fail. */
}
- if (!TYPE_NOTHROW_P (type))
+ if (!INDIRECT_TYPE_P (type)
+ || !TYPE_NOTHROW_P (TREE_TYPE (type)))
return fn;
}
which leaves us with
FAIL: g++.dg/warn/noeffect4.C -std=gnu++98 (test for warnings, line 84)
FAIL: g++.dg/warn/noeffect4.C -std=gnu++14 (test for warnings, line 84)
FAIL: g++.dg/warn/noeffect4.C -std=gnu++17 (test for warnings, line 84)
FAIL: g++.dg/warn/noeffect4.C -std=gnu++2a (test for warnings, line 84)
not diagnosing
sizeof (x++);// { dg-warning "no effect" }
not sure what should happen to the x++ side-effect (throwing).
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 --- Comment #9 from Harald van Dijk --- (In reply to Richard Biener from comment #8) > It has been consensus that throwing exceptions and const/pure are different > concepts that co-exist. See for example the recent discussion at > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/569435.html Thanks, based on that I have created PR101376.
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 --- Comment #8 from Richard Biener --- (In reply to Harald van Dijk from comment #4) > The documentation for the pure attribute refers to "functions that have no > observable effects on the state of the program other than to return a value" > which implies not throwing exceptions, the Wsuggest-attribute=pure text says > that pure functions have to return normally, and the presence of throw > statements suppresses the compiler's suggestion to mark functions as pure. > This function throws, so should the fact that it is marked pure not simply > make the whole thing undefined? It has been consensus that throwing exceptions and const/pure are different concepts that co-exist. See for example the recent discussion at https://gcc.gnu.org/pipermail/gcc-patches/2021-May/569435.html
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 Richard Biener changed: What|Removed |Added CC||jason at gcc dot gnu.org --- Comment #7 from Richard Biener --- Jason, any idea?
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 --- Comment #6 from Richard Biener --- Created attachment 51116 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51116=edit patch I was testing I was testing this patch.
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409
--- Comment #5 from Richard Biener ---
Hmm, doesn't quite work.
FAIL: g++.dg/cpp0x/sfinae19.C -std=c++14 (internal compiler error)
FAIL: g++.dg/cpp0x/sfinae22.C -std=c++14 (internal compiler error)
FAIL: g++.dg/cpp1y/pr61636-2.C -std=c++14 (internal compiler error)
FAIL: g++.dg/warn/noeffect4.C -std=gnu++14 (test for errors, line 79)
FAIL: g++.dg/warn/noeffect4.C -std=gnu++14 (test for warnings, line 80)
FAIL: g++.dg/warn/noeffect4.C -std=gnu++14 (test for warnings, line 82)
FAIL: g++.dg/warn/noeffect4.C -std=gnu++14 (test for warnings, line 83)
FAIL: g++.dg/warn/noeffect4.C -std=gnu++14 (test for warnings, line 84)
FAIL: g++.dg/warn/noeffect4.C -std=gnu++14 (test for warnings, line 85)
FAIL: g++.dg/warn/noeffect4.C -std=gnu++14 (test for warnings, line 88)
FAIL: g++.dg/warn/noeffect4.C -std=gnu++14 (internal compiler error)
FAIL: g++.dg/warn/noeffect4.C -std=gnu++14 (test for excess errors)
the ICE is
/home/rguenther/src/trunk/gcc/testsuite/g++.dg/cpp0x/sfinae19.C:8:30: internal
compiler error: in check_noexcept_r, at cp/except.c:1053^M
if ((code == CALL_EXPR && CALL_EXPR_FN (t))
|| code == AGGR_INIT_EXPR)
{
/* We can only use the exception specification of the called function
for determining the value of a noexcept expression; we can't use
TREE_NOTHROW, as it might have a different value in another
translation unit, creating ODR problems.
We could use TREE_NOTHROW (t) for !TREE_PUBLIC fns, though... */
tree fn = cp_get_callee (t);
if (concept_check_p (fn))
return NULL_TREE;
tree type = TREE_TYPE (fn);
gcc_assert (INDIRECT_TYPE_P (type));
where the type is likely the lang_type in:
VOID
align:1 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x766851f8
pointer_to_this
reference_to_this >
whatever that exactly is, it isn't a reference or pointer type.
processing_template_decl is true in this context, but if we assume
that in this context expr_noexcept_p is true we probably miss diagnostics.
Jason, any idea?
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 Harald van Dijk changed: What|Removed |Added CC||harald at gigawatt dot nl --- Comment #4 from Harald van Dijk --- The documentation for the pure attribute refers to "functions that have no observable effects on the state of the program other than to return a value" which implies not throwing exceptions, the Wsuggest-attribute=pure text says that pure functions have to return normally, and the presence of throw statements suppresses the compiler's suggestion to mark functions as pure. This function throws, so should the fact that it is marked pure not simply make the whole thing undefined?
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 --- Comment #3 from Richard Biener --- So I'm looking again at this. For the simple cases I have at hand we can fix the C++ FE issue with diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c index d035e611be4..81097942449 100644 --- a/gcc/cp/cvt.c +++ b/gcc/cp/cvt.c @@ -1645,7 +1645,12 @@ convert_to_void (tree expr, impl_conv_void implicit, tsub st_flags_t complain) } expr = build1 (CONVERT_EXPR, void_type_node, expr); } - if (! TREE_SIDE_EFFECTS (expr)) + if (! TREE_SIDE_EFFECTS (expr) + && ! (CONVERT_EXPR_P (expr) + && TREE_CODE (TREE_OPERAND (expr, 0)) == CALL_EXPR + && ! TREE_NOTHROW (TREE_OPERAND (expr, 0))) + && ! (TREE_CODE (expr) == CALL_EXPR + && ! TREE_NOTHROW (expr))) expr = void_node; return expr; } of course that relies on seeing f() or (void)f(), it already fails with (void)(1 + f()). We also diagnose such stmts as having no effect: t.ii: In function 'int bar(int*, int)': t.ii:13:10: warning: statement has no effect [-Wunused-value] 13 |1 + foo (n); |~~^ there's expr_noexcept_p that seems to work here even for 1 + foo () but I'm not sure it's the correct tool. But it would simplify a fix to diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c index d035e611be4..4885a7c6584 100644 --- a/gcc/cp/cvt.c +++ b/gcc/cp/cvt.c @@ -1645,7 +1645,7 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain) } expr = build1 (CONVERT_EXPR, void_type_node, expr); } - if (! TREE_SIDE_EFFECTS (expr)) + if (! TREE_SIDE_EFFECTS (expr) && expr_noexcept_p (expr, 0)) expr = void_node; return expr; } I'm giving that full testing.
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409 --- Comment #2 from Richard Biener --- After the PR100394 fix this is now the prevailing issue.
[Bug c++/100409] C++ FE elides pure throwing call
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100409
Richard Biener changed:
What|Removed |Added
Keywords||wrong-code
--- Comment #1 from Richard Biener ---
Runtime testcase that aborts:
int x, y;
int __attribute__((pure,noinline)) foo () { if (x) throw 1; return y; }
int __attribute__((noinline)) foo2 () { foo (); return y; }
int __attribute__((noinline)) bar()
{
int a[2];
x = 1;
try {
int res = foo2 ();
a[0] = res;
} catch (...) {
return 0;
}
return 1;
}
int main()
{
if (bar ())
__builtin_abort ();
return 0;
}
