> On Mar 16, 2026, at 10:41 AM, Andrew Pinski <[email protected]>
> wrote:
>
> On Mon, Mar 16, 2026 at 9:06 AM Chris Copeland <[email protected]> wrote:
>>
>> This fails on g++.dg/guality/pr55665.C
>>
>>> Produces 1 regression:
>>> |
>>> | regressions.sum:
>>> | Running g++:g++.dg/guality/guality.exp ...
>>> | FAIL: g++.dg/guality/pr55665.C -O2 -flto -fno-use-linker-plugin
>>> -flto-partition=none line 23 p == 40
>>
>> Looking at that test file closely, it's not clear that its "memory" clobber
>> is actually
>> needed for what it's testing. The effect of the patch in this case changes
>> whether
>> gdb shows variable's value, presumably because it's now possibly modified.
>> The behavior of the test program is unchanged.
>> Any objections to removing the "memory" clobber here?
>
> No I think that would be wrong. You are going to have to debug why the
> debug information is now messed up.
> Seems like it should not be.
>
> Thanks,
> Andrew Pinski
See below for more details of the pr55665.C test failure and a reproducer of
the same failure on master with slightly different code without any memory
clobbers. Let me know if this is sufficient to proceed with modifying the
pr55665.C test to not use a memory clobber. On closer inspection, the memory
clobber was being ignored in this test prior to this patch anyway, so removing
it preserves the behavior of the test more faithfully than leaving it in.
Details re: pr55665.C:
foo has a memory clobber, and because that is now honored with this patch,
A's constructor needs to re-load p after calling foo, regardless of whether p's
initialization occurred in that invocation of A::A or a prior one.
Roughly, the compiled code for A::A with this patch looks like:
ldr x2, [guard]
tbnz w2, #0, .Lmerge
mov w2, #40
str w2, [p]
mov x2, #1
str x2, [guard]
.Lmerge:
ldr w0, [p]
bl foo(int)
ldr w0, [p]
add w0, w0, #1
str w0, [p]
Before, when the "memory" clobber was ignored, we actually have two locations
where the call to foo is made, because in the case where p needs to be
initialized still, we are just using w0 for the subsequent operations,
rather than re-loading p.
ldr x2, [guard]
tbnz w2, #0, .Lalready_init
mov x2, #1
str x2, [guard]
mov w0, #40
bl foo(int)
mov w0, #41
str w0, [p]
ret
.Lalready_init:
ldr w0, [p]
bl foo(int)
add w0, w0, #1
str w0, [p]
Now, the test fails because the DWARF for this program has p in a different
stub subprogram tag that does not have a valid PC range. You can get the same
behavior if you remove the "memory" clobber and instead make p volatile, which
has a similar effect on the compiled code and leads to the same misplaced
DW_TAG_variable.
This is quite complex and I am not sure why the DWARF is being generated
incorrectly here, but this is a latent bug that can be reproduced without any
inline assembly or clobbers.
Here is a modified version of the same test that fails on master.
Test run output:
Running /src/gcc/gcc/testsuite/g++.dg/guality/guality.exp ...
FAIL: g++.dg/guality/pr55665-volatile.C -O2 -flto -fno-use-linker-plugin
-flto-partition=none line 27 p == 40
Test log snippet:
Breakpoint 1, A::A (...) at .../pr55665-volatile.C:27
27 foo (p); // { dg-final { gdb-test 27 "p" "40" } }
$1 = <optimized out>
$2 = 40
<optimized out> != 40
pr55665-volatile.C:
// { dg-do run }
// { dg-options "-g" }
extern "C" void abort ();
struct A
{
A (int);
int a;
};
__attribute__((noinline, noclone)) int
bar (void)
{
return 40;
}
__attribute__((noinline, noclone)) void
foo (int x)
{
__asm volatile ("" : : "r" (x));
}
A::A (int x)
{
static volatile int p = bar ();
foo (p); // { dg-final { gdb-test 27 "p" "40" } }
a = p + 1;
p = a;
}
int
main ()
{
A a (42);
if (a.a != 41)
abort ();
}
>
>>
>>> On Mar 15, 2026, at 6:56 PM, Chris Copeland <[email protected]> wrote:
>>>
>>> Record the presence of asm memory clobbers in ipa_fn_summary and use it
>>> in ipa-reference's analyze_function to mark all module statics as read
>>> and written. Move pass_ipa_reference before pass_ipa_free_fn_summary so
>>> the summary is still available when needed.
>>>
>>> gcc/ChangeLog:
>>>
>>> PR ipa/124218
>>> * ipa-fnsummary.h (ipa_fn_summary): Add asm_memory_clobber field.
>>> * ipa-fnsummary.cc (analyze_function_body): Set asm_memory_clobber
>>> when a GIMPLE_ASM with a memory clobber is found.
>>> (ipa_merge_fn_summary_after_inlining): Merge asm_memory_clobber.
>>> (inline_read_section): Stream in asm_memory_clobber.
>>> (ipa_fn_summary_write): Stream out asm_memory_clobber.
>>> * ipa-reference.cc (analyze_function): Use asm_memory_clobber from
>>> fn summary to mark all module statics as read and written.
>>> * passes.def: Move pass_ipa_reference before pass_ipa_free_fn_summary.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR ipa/124218
>>> * gcc.dg/ipa/pr124218.c: New test.
>>> ---
>>> gcc/ChangeLog | 13 +++++++++++++
>>> gcc/ipa-fnsummary.cc | 9 +++++++++
>>> gcc/ipa-fnsummary.h | 6 +++++-
>>> gcc/ipa-reference.cc | 13 +++++++++++++
>>> gcc/passes.def | 2 +-
>>> gcc/testsuite/ChangeLog | 5 +++++
>>> gcc/testsuite/gcc.dg/ipa/pr124218.c | 30 +++++++++++++++++++++++++++++
>>> 7 files changed, 76 insertions(+), 2 deletions(-)
>>> create mode 100644 gcc/testsuite/gcc.dg/ipa/pr124218.c
>>>
>>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
>>> index 1a6bab7bc65..57c67037c77 100644
>>> --- a/gcc/ChangeLog
>>> +++ b/gcc/ChangeLog
>>> @@ -1,3 +1,16 @@
>>> +2026-03-15 Chris Copeland <[email protected]>
>>> +
>>> + PR ipa/124218
>>> + * ipa-fnsummary.h (ipa_fn_summary): Add asm_memory_clobber field.
>>> + * ipa-fnsummary.cc (analyze_function_body): Set asm_memory_clobber
>>> + when a GIMPLE_ASM with a memory clobber is found.
>>> + (ipa_merge_fn_summary_after_inlining): Merge asm_memory_clobber.
>>> + (inline_read_section): Stream in asm_memory_clobber.
>>> + (ipa_fn_summary_write): Stream out asm_memory_clobber.
>>> + * ipa-reference.cc (analyze_function): Use asm_memory_clobber from
>>> + fn summary to mark all module statics as read and written.
>>> + * passes.def: Move pass_ipa_reference before pass_ipa_free_fn_summary.
>>> +
>>> 2026-03-14 Jakub Jelinek <[email protected]>
>>>
>>> PR other/124508
>>> diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc
>>> index e187231dfb6..2e1387ec746 100644
>>> --- a/gcc/ipa-fnsummary.cc
>>> +++ b/gcc/ipa-fnsummary.cc
>>> @@ -3200,6 +3200,11 @@ analyze_function_body (struct cgraph_node *node,
>>> bool early)
>>> }
>>> }
>>>
>>> + if (!info->asm_memory_clobber
>>> + && gimple_code (stmt) == GIMPLE_ASM
>>> + && gimple_asm_clobbers_memory_p (as_a <gasm *> (stmt)))
>>> + info->asm_memory_clobber = true;
>>> +
>>> /* For target specific information, we want to scan all statements
>>> rather than those statements with non-zero weights, to avoid
>>> missing to scan something interesting for target information,
>>> @@ -4492,6 +4497,7 @@ ipa_merge_fn_summary_after_inlining (struct
>>> cgraph_edge *edge)
>>> toplev_predicate = true;
>>>
>>> info->fp_expressions |= callee_info->fp_expressions;
>>> + info->asm_memory_clobber |= callee_info->asm_memory_clobber;
>>> info->target_info |= callee_info->target_info;
>>>
>>> if (callee_info->conds)
>>> @@ -4838,11 +4844,13 @@ inline_read_section (struct lto_file_decl_data
>>> *file_data, const char *data,
>>> {
>>> info->inlinable = bp_unpack_value (&bp, 1);
>>> info->fp_expressions = bp_unpack_value (&bp, 1);
>>> + info->asm_memory_clobber = bp_unpack_value (&bp, 1);
>>> if (!lto_stream_offload_p)
>>> info->target_info = streamer_read_uhwi (&ib);
>>> }
>>> else
>>> {
>>> + bp_unpack_value (&bp, 1);
>>> bp_unpack_value (&bp, 1);
>>> bp_unpack_value (&bp, 1);
>>> if (!lto_stream_offload_p)
>>> @@ -5084,6 +5092,7 @@ ipa_fn_summary_write (void)
>>> bp = bitpack_create (ob->main_stream);
>>> bp_pack_value (&bp, info->inlinable, 1);
>>> bp_pack_value (&bp, info->fp_expressions, 1);
>>> + bp_pack_value (&bp, info->asm_memory_clobber, 1);
>>> streamer_write_bitpack (&bp);
>>> if (!lto_stream_offload_p)
>>> streamer_write_uhwi (ob, info->target_info);
>>> diff --git a/gcc/ipa-fnsummary.h b/gcc/ipa-fnsummary.h
>>> index de7b7f61cb1..896cb6c1725 100644
>>> --- a/gcc/ipa-fnsummary.h
>>> +++ b/gcc/ipa-fnsummary.h
>>> @@ -126,7 +126,8 @@ public:
>>> ipa_fn_summary ()
>>> : min_size (0),
>>> inlinable (false), single_caller (false),
>>> - fp_expressions (false), safe_to_inline_to_always_inline (0),
>>> + fp_expressions (false), asm_memory_clobber (false),
>>> + safe_to_inline_to_always_inline (0),
>>> target_info (0), estimated_stack_size (false),
>>> time (0), conds (NULL),
>>> size_time_table (), call_size_time_table (vNULL),
>>> @@ -141,6 +142,7 @@ public:
>>> : min_size (s.min_size),
>>> inlinable (s.inlinable), single_caller (s.single_caller),
>>> fp_expressions (s.fp_expressions),
>>> + asm_memory_clobber (s.asm_memory_clobber),
>>> target_info (s.target_info),
>>> estimated_stack_size (s.estimated_stack_size),
>>> time (s.time), conds (s.conds), size_time_table (),
>>> @@ -165,6 +167,8 @@ public:
>>> unsigned int single_caller : 1;
>>> /* True if function contains any floating point expressions. */
>>> unsigned int fp_expressions : 1;
>>> + /* True if function contains an asm statement that clobbers memory. */
>>> + unsigned int asm_memory_clobber : 1;
>>> /* Cache for analysis of can_early_inline_edge_p. */
>>> unsigned int safe_to_inline_to_always_inline : 2;
>>> /* Like fp_expressions field above, but it's to hold some target specific
>>> diff --git a/gcc/ipa-reference.cc b/gcc/ipa-reference.cc
>>> index c5699312c8f..12b593b0f50 100644
>>> --- a/gcc/ipa-reference.cc
>>> +++ b/gcc/ipa-reference.cc
>>> @@ -50,6 +50,11 @@ along with GCC; see the file COPYING3. If not see
>>> #include "ipa-reference.h"
>>> #include "alloc-pool.h"
>>> #include "symbol-summary.h"
>>> +#include "tree-vrp.h"
>>> +#include "sreal.h"
>>> +#include "ipa-cp.h"
>>> +#include "ipa-prop.h"
>>> +#include "ipa-fnsummary.h"
>>>
>>> /* The static variables defined within the compilation unit that are
>>> loaded or stored directly by function that owns this structure. */
>>> @@ -507,6 +512,7 @@ analyze_function (struct cgraph_node *fn)
>>> {
>>> ipa_reference_local_vars_info_t local;
>>> struct ipa_ref *ref = NULL;
>>> + ipa_fn_summary *sum;
>>> int i;
>>> tree var;
>>>
>>> @@ -548,6 +554,13 @@ analyze_function (struct cgraph_node *fn)
>>> }
>>> }
>>>
>>> + sum = ipa_fn_summaries->get (fn);
>>> + if (sum && sum->asm_memory_clobber)
>>> + {
>>> + local->statics_read = all_module_statics;
>>> + local->statics_written = all_module_statics;
>>> + }
>>> +
>>> if (fn->cannot_return_p ())
>>> bitmap_clear (local->statics_written);
>>> }
>>> diff --git a/gcc/passes.def b/gcc/passes.def
>>> index cdddb87302f..8344ea1a584 100644
>>> --- a/gcc/passes.def
>>> +++ b/gcc/passes.def
>>> @@ -176,8 +176,8 @@ along with GCC; see the file COPYING3. If not see
>>> NEXT_PASS (pass_ipa_locality_cloning);
>>> NEXT_PASS (pass_ipa_pure_const);
>>> NEXT_PASS (pass_ipa_modref);
>>> - NEXT_PASS (pass_ipa_free_fn_summary, false /* small_p */);
>>> NEXT_PASS (pass_ipa_reference);
>>> + NEXT_PASS (pass_ipa_free_fn_summary, false /* small_p */);
>>> /* This pass needs to be scheduled after any IP code duplication. */
>>> NEXT_PASS (pass_ipa_single_use);
>>> /* Comdat privatization come last, as direct references to comdat local
>>> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
>>> index f0b92db69fc..0337f6192f6 100644
>>> --- a/gcc/testsuite/ChangeLog
>>> +++ b/gcc/testsuite/ChangeLog
>>> @@ -1,3 +1,8 @@
>>> +2026-03-15 Chris Copeland <[email protected]>
>>> +
>>> + PR ipa/124218
>>> + * gcc.dg/ipa/pr124218.c: New test.
>>> +
>>> 2026-03-14 Jakub Jelinek <[email protected]>
>>>
>>> PR c++/124399
>>> diff --git a/gcc/testsuite/gcc.dg/ipa/pr124218.c
>>> b/gcc/testsuite/gcc.dg/ipa/pr124218.c
>>> new file mode 100644
>>> index 00000000000..67e95914d6b
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/ipa/pr124218.c
>>> @@ -0,0 +1,30 @@
>>> +/* { dg-do run { target arm*-*-eabi* } } */
>>> +/* { dg-options "-O1 -fwhole-program" } */
>>> +
>>> +/* PR ipa/124218: ipa-reference must honor memory clobbers in inline asm.
>>> */
>>> +
>>> +int flag;
>>> +
>>> +__attribute__ ((used))
>>> +void
>>> +asm_set (void)
>>> +{
>>> + flag = 1;
>>> +}
>>> +
>>> +__attribute__ ((noinline))
>>> +static void
>>> +clobber_and_set (void)
>>> +{
>>> + __asm__ volatile ("bl asm_set" ::: "r0", "r1", "r2", "r3",
>>> + "ip", "lr", "memory", "cc");
>>> +}
>>> +
>>> +int main (void)
>>> +{
>>> + flag = 0;
>>> + clobber_and_set ();
>>> + if (!flag)
>>> + __builtin_abort ();
>>> + return 0;
>>> +}
>>> --
>>> 2.53.0