[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400 --- Comment #17 from GCC Commits --- The trunk branch has been updated by Jason Merrill : https://gcc.gnu.org/g:ee636671898c6bce63d9b2a698007b609aabbfe8 commit r16-919-gee636671898c6bce63d9b2a698007b609aabbfe8 Author: Jason Merrill Date: Fri May 23 09:41:25 2025 -0400 fold: DECL_VALUE_EXPR isn't simple [PR120400] This PR noted that fold_truth_andor was wrongly changing && to & where the RHS is a VAR_DECL with DECL_VALUE_EXPR; we can't assume that such can be evaluated unconditionally. To be more precise we could recurse into DECL_VALUE_EXPR, but that doesn't seem worth bothering with since typical uses involve a COMPONENT_REF, which is not simple. PR c++/120400 gcc/ChangeLog: * fold-const.cc (simple_operand_p): False for vars with DECL_VALUE_EXPR.
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400 --- Comment #16 from Iain Sandoe --- (In reply to Jason Merrill from comment #15) > Created attachment 61520 [details] > updated fix > > Here's a better version of the patch, also sent to gcc-patches. replied on-list, this also resolves the issue (with the typo fixes at comment #14)
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400 Jason Merrill changed: What|Removed |Added Attachment #61500|0 |1 is obsolete|| --- Comment #15 from Jason Merrill --- Created attachment 61520 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=61520&action=edit updated fix Here's a better version of the patch, also sent to gcc-patches.
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400 Iain Sandoe changed: What|Removed |Added Ever confirmed|0 |1 Last reconfirmed||2025-05-23 Status|UNCONFIRMED |NEW --- Comment #14 from Iain Sandoe --- (In reply to Jason Merrill from comment #13) > Created attachment 61500 [details] > possible fix > > Does this fix the issue? Yes, it does .. that's the kind of thing I was hoping for ... we also need the changes below - which I'll post separately. @@ -5017,7 +5020,7 @@ cp_coroutine_transform::build_ramp_function () tree frame_cleanup = push_stmt_list (); tree do_fr_cleanup = build1_loc (loc, TRUTH_NOT_EXPR, boolean_type_node, iarc_x); - do_fr_cleanup = build2_loc (loc, TRUTH_AND_EXPR, boolean_type_node, + do_fr_cleanup = build2_loc (loc, TRUTH_ANDIF_EXPR, boolean_type_node, coro_before_return, do_fr_cleanup); r = build3 (COND_EXPR, void_type_node, do_fr_cleanup, delete_frame_call, void_node); @@ -5109,7 +5112,7 @@ cp_coroutine_transform::build_ramp_function () tree do_cleanup = build1_loc (loc, TRUTH_NOT_EXPR, boolean_type_node, iarc_x); do_cleanup - = build2_loc (loc, TRUTH_AND_EXPR, boolean_type_node, + = build2_loc (loc, TRUTH_ANDIF_EXPR, boolean_type_node, coro_before_return, do_cleanup); r = build3_loc (loc, COND_EXPR, void_type_node, do_cleanup, parm.fr_copy_dtor, void_node); @@ -5169,7 +5172,7 @@ cp_coroutine_transform::build_ramp_function () tree promise_cleanup = push_stmt_list (); tree do_cleanup = build1_loc (loc, TRUTH_NOT_EXPR, boolean_type_node, iarc_x); - do_cleanup = build2_loc (loc, TRUTH_AND_EXPR, boolean_type_node, + do_cleanup = build2_loc (loc, TRUTH_ANDIF_EXPR, boolean_type_node, coro_before_return, do_cleanup); r = build3 (COND_EXPR, void_type_node, do_cleanup, promise_dtor, void_node);
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400 --- Comment #13 from Jason Merrill --- Created attachment 61500 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=61500&action=edit possible fix Does this fix the issue?
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400
--- Comment #12 from Iain Sandoe ---
(In reply to Richard Biener from comment #11)
> D_V_E? DECL_VALUE_EXPR?
yes..
> I guess we should indeed treat those as having
> side-effects.
struct F {
bool b;
};
int foo (F *a)
{
if (a && !a->b)
return 6;
return 42;
}
Works as expected - but I do not see TREE_SIDE_EFFECTS set on any part of the
component ref expression.
I was going to investigate "looking through" expressions with a D_V_E and
applying the rules to the latter .. but not got to that yet.
> But without actually setting TREE_SIDE_EFFECTS that's hard to
> properly propagate everywhere, so yeah - I suggest doing that.
Hmm - seems a bit of a big hammer .. but.
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400 --- Comment #11 from Richard Biener --- D_V_E? DECL_VALUE_EXPR? I guess we should indeed treat those as having side-effects. But without actually setting TREE_SIDE_EFFECTS that's hard to properly propagate everywhere, so yeah - I suggest doing that.
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400 --- Comment #10 from Iain Sandoe --- (In reply to Iain Sandoe from comment #8) > (In reply to Andrew Pinski from comment #7) > > I think if you set TREE_SIDE_EFFECTS on the decls that have a D_V_E, then it > > might just work. > > i can also try that (but I'd want to avoid doing something that's possibly > fragile). Setting that on the decls does "work" (as I read tree.h that is effectively making those vars "volatile"). I'll do this for now, but would be interested if there's some better strategy (other than doing two nested ifs).
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400 --- Comment #7 from Andrew Pinski --- I think if you set TREE_SIDE_EFFECTS on the decls that have a D_V_E, then it might just work.
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400
--- Comment #9 from Andrew Pinski ---
(In reply to Iain Sandoe from comment #8)
> (In reply to Andrew Pinski from comment #7)
> > I think if you set TREE_SIDE_EFFECTS on the decls that have a D_V_E, then it
> > might just work.
>
> i can also try that (but I'd want to avoid doing something that's possibly
> fragile).
>
> note that:
>
> int foo (bool *a)
> {
> if (a && !*a)
> return 6;
> return 42;
> }
>
> behaves as expected.
Right because INDIRECT_REF has TREE_SIDE_EFFECTS set on it when it is built.
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400
--- Comment #8 from Iain Sandoe ---
(In reply to Andrew Pinski from comment #7)
> I think if you set TREE_SIDE_EFFECTS on the decls that have a D_V_E, then it
> might just work.
i can also try that (but I'd want to avoid doing something that's possibly
fragile).
note that:
int foo (bool *a)
{
if (a && !*a)
return 6;
return 42;
}
behaves as expected.
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400 --- Comment #6 from Andrew Pinski --- (In reply to Iain Sandoe from comment #5) > > I must say it's also somewhat odd that the folder chooses the more expensive > condition to come first - but then if there's no short-circuiting then all > the ops will be done anyway. That comes from tree_swap_operands_p: /* Put variables last. */ if (DECL_P (arg1)) return false; if (DECL_P (arg0)) return true;
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400 --- Comment #5 from Iain Sandoe --- (In reply to Iain Sandoe from comment #4) > (In reply to Andrew Pinski from comment #3) > > that is TRUTH_ANDIF_EXPR being converted into TRUTH_AND_EXPR in fold because > > fold thinks coro_before_return and cond don't have side effects as they are > > normal decls at the point fold is called. > > hmm .. perhaps if a DECL has a D_V_E that should be blocked? > (one of the decls is "normal" coro_before_return ,,, bu the other is not - > it has a D_V_E pointing to the coroutine frame, and the folder probably > should not be reasoning about that). > > ? I must say it's also somewhat odd that the folder chooses the more expensive condition to come first - but then if there's no short-circuiting then all the ops will be done anyway.
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400 --- Comment #4 from Iain Sandoe --- (In reply to Andrew Pinski from comment #3) > that is TRUTH_ANDIF_EXPR being converted into TRUTH_AND_EXPR in fold because > fold thinks coro_before_return and cond don't have side effects as they are > normal decls at the point fold is called. hmm .. perhaps if a DECL has a D_V_E that should be blocked? (one of the decls is "normal" coro_before_return ,,, bu the other is not - it has a D_V_E pointing to the coroutine frame, and the folder probably should not be reasoning about that). ?
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400 Andrew Pinski changed: What|Removed |Added Component|middle-end |c++ --- Comment #3 from Andrew Pinski --- that is TRUTH_ANDIF_EXPR being converted into TRUTH_AND_EXPR in fold because fold thinks coro_before_return and cond don't have side effects as they are normal decls at the point fold is called.
[Bug c++/120400] C++ FE optimisations reorder && operands.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120400
Iain Sandoe changed:
What|Removed |Added
Version|15.0|16.0
--- Comment #1 from Iain Sandoe ---
trivial reproducer:
int foo (bool a, bool b)
{
if (a && !b)
return 6;
return 42;
}
I seee the same back to 10.5 (on x86_64-darwin21).
