> 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


Reply via email to