> On Mar 17, 2026, at 11:44 PM, Chris Copeland <[email protected]> wrote:
>
>> 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.
Correction: there is still inline asm in the reproducer, just no 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