> On Wed, Mar 4, 2026 at 1:41 AM Andrew Pinski
> <[email protected]> wrote:
> >
> > When merging the modref summary after inlining, we merge all of the flags
> > of the outer functions that was inlined into. But things go wrong as
> > now the flags includes both ECF_NORETURN and ECF_NOTHROW. This happens
> > because the function which was being inlined had ECF_NOTHROW while caller
> > had ECF_NORETURN. When both of these are set, ignore_stores_p and
> > ignore_nondeterminism_p
> > return true. But in this case the inner function is still just nothrow
> > and not noreturn.
> > Basically we only want to combine in the CONST/PURE/NOVOPS related flags
> > instead.
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > PR tree-optimization/120987
> >
> > gcc/ChangeLog:
> >
> > * ipa-modref.cc (ipa_merge_modref_summary_after_inlining): Mask
> > off non const/pure/novops related flags when combining of outer
> > functions.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/torture/pr120987-1.C: New test.
> >
> > Signed-off-by: Andrew Pinski <[email protected]>
> > ---
> > gcc/ipa-modref.cc | 11 ++++-
> > gcc/testsuite/g++.dg/torture/pr120987-1.C | 57 +++++++++++++++++++++++
> > 2 files changed, 66 insertions(+), 2 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/torture/pr120987-1.C
> >
> > diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc
> > index fc00acecfce..3158e9757fc 100644
> > --- a/gcc/ipa-modref.cc
> > +++ b/gcc/ipa-modref.cc
> > @@ -5345,9 +5345,16 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge
> > *edge)
> > int flags = flags_from_decl_or_type (edge->callee->decl);
> > /* Combine in outer flags. */
> > cgraph_node *n;
> > + /* Only combine const/pure/novops related flags.
> > + NoReturn and NoThrow cannot be combined.
> > + An outer noreturn function might throw but an inner might be marked
> > as nothrow. */
>
> Also in an inline chain A -> B -> C, B might be marked as noreturn but
> that does not mean
> A is noreturn. So when working on the last bug in this function I was
> very confused (but that
> bug turned out to be unrelated, so I didn't dare touch this code).
Indeed this looks like oversight. AFAIK that loop was there already at
a time we was only collection load/store vector before we started using
NORETURN/NOTHROW attributes and determinism and I did not notice the
problem while adding logic for it.
The loop should compute "effective" const/pure/novops flags of the
inlined function from the point of view of caller of n->decl (as if they
were attributes of it), which should be combination of the flags on
inline chain.
For ECF_CONST/PURE/NOVOPS we want the strongest flag we meet on the way
(so merging via or should do the job). For ECF_LOOPING_CONST_OR_PURE we
could do better since for chain A->B->C where B is ECF_PURE |
ECF_LOOPING_CONST_OR_PURE and A is ECF_CONST, we know that the whole
chain should behave as ECF_CONST since the outer flag guarantees that
the function is not infinite. Doing | is conservatively correct.
>
> > + int flags_mask;
> > + flags_mask = ECF_CONST | ECF_PURE | ECF_NOVOPS |
> > ECF_LOOPING_CONST_OR_PURE;
> > for (n = edge->caller; n->inlined_to; n = n->callers->caller)
> > - flags |= flags_from_decl_or_type (n->decl);
> > - flags |= flags_from_decl_or_type (n->decl);
> > + flags |= flags_from_decl_or_type (n->decl) & flags_mask;
> > +
> > + /* Combine in the outer most. */
> > + flags |= flags_from_decl_or_type (n->decl) & flags_mask;
We do not need to mask out the flags for the outermost.
(if that one is NORETURN & NOTHROW, no stores in the whole function will
be visible to callers).
For computing ignore_stores we can just or together ignore stores on the
while inline chain. (If any of those function can not return to caller,
the stores of the function being inlined can be ignored)
> > bool ignore_stores = ignore_stores_p (edge->caller->decl, flags);
>
> I also noticed we pass edge->caller->decl here (B), not A that B is
> inlined into. Though
> as we only check flag_exceptions on that decl and inlining should not
> happen on a
> flag_exceptions change boundary this is probably harmless.
It should not matter, but passing n->decl is correct one for the outer
flags.
>
> Note we're using 'flags' for many checks in the following code. I
> think this is all mostly
> "optimistic" merging, aka if a caller was modref analyzed to some ECF_* then
> the
> actual instance of a callee (thus when inlined) has to adhere to that,
> otherwise the
> modref analysis was wrong. But I do not see how this should allow for
Yes, also it is quite possible that const function contains some non-const calls
in it. For example, it may have a local array and do some load/stores
via function calls on it.
> the reverse
> merging of callee flags to caller flags. For the ignore_stores_p it
> does effectively
> merge caller flags to the callee of the inlined edge. In this context
> I do not understand:
>
> > + An outer noreturn function might throw but an inner might be marked
> > as nothrow. */
>
> If A is noreturn, but not nothrow, B is nothrow, then for the purpose
> of inlining B -> C we
> are currently treating the caller as noreturn + nothrow which should
> be OK for the stores
> recorded on 'C'? Not. But then the whole scheme falls apart again.
Yes, we want at least one of A, B, C to be noreturn & nothrow
and not to combine the bits together and check if result is
noreturn & nothrow
Honza
>
> As said, I'm confused by this code. In my previous attempt I just wiped it
> all.
>
> Meaning, I'll defer to Honza.
>
> This code needs a big fat comment explaining what it does.
>
> Thanks for trying.
> Richard.
>
> >
> > if (!callee_info && to_info)
> > diff --git a/gcc/testsuite/g++.dg/torture/pr120987-1.C
> > b/gcc/testsuite/g++.dg/torture/pr120987-1.C
> > new file mode 100644
> > index 00000000000..4209679bc03
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/torture/pr120987-1.C
> > @@ -0,0 +1,57 @@
> > +// { dg-do run { target c++11 } }
> > +// { dg-skip-if "requires hosted libstdc++ for string/memory" { !
> > hostedlib } }
> > +// PR tree-optimization/120987
> > +
> > +#include <memory>
> > +#include <string>
> > +#include <cstdlib>
> > +
> > +#define ERROR_STRING "012345678901234567"
> > +
> > +struct gdb_exception
> > +{
> > + gdb_exception (const char *s)
> > + : message (std::make_shared<std::string> (s))
> > + {}
> > +
> > + explicit gdb_exception (gdb_exception &&other) noexcept
> > + : message (std::move (other.message))
> > + {
> > + volatile int a = 1;
> > + if (a != 1)
> > + abort ();
> > + }
> > +
> > +
> > + std::shared_ptr<std::string> message;
> > +};
> > +
> > +void __attribute__((noinline, noclone))
> > +throw_exception (gdb_exception &&exception)
> > +{
> > + throw gdb_exception (std::move (exception));
> > +}
> > +
> > +static void __attribute__((noinline, noclone))
> > +parse_linespec (void)
> > +{
> > + struct gdb_exception file_exception (ERROR_STRING);
> > + throw_exception (std::move (file_exception));
> > +}
> > +
> > +int
> > +main (void)
> > +{
> > + try
> > + {
> > + parse_linespec ();
> > + }
> > + catch (const gdb_exception &e)
> > + {
> > + if (*e.message != ERROR_STRING)
> > + __builtin_abort();
> > + return 0;
> > + }
> > +
> > + __builtin_abort();
> > +}
> > --
> > 2.43.0
> >
