> > On Mar 5, 2026, at 12:13 AM, Michal Jires <[email protected]> wrote: > > > > On Wed, 2026-03-04 at 16:39:12 -0800, Chris Copeland wrote: > >>> > >>>>> +/* PR ipa/124218: ipa-reference must honor memory clobbers in inline > >>>>> asm. */ > >>>>> + > >>>>> +int flag; > >>>>> + > >>>>> +__attribute__ ((noinline)) > >>>>> +static void > >>>>> +clobber_and_set (void) > >>>>> +{ > >>>>> + __asm__ volatile ("ldr r0, =flag\n\t" > >>>>> + "mov r1, #1\n\t" > >>>>> + "str r1, [r0]" > >>>>> + ::: "r0", "r1", "memory"); > >>> > >>> At least withtout new Michal's heuristics, for making this safe with LTO > >>> you need used attribute on flag and that disables ipa-reference on it. > >> > >> This was discussed some on the bug report, but in the real code this > >> clobber is at the location where a context switch is triggered, which > >> eventually leads to another task executing that modifies flag. I wrote the > >> test this way because it's the simplest thing I could come up with to > >> represent that situation. Would an unrelated and not-called function that > >> does modify the flag obviate the need for ((used)) in this test, even > >> without forthcoming changes? > >> > > > > The issue is with hardcoding symbols into the asm statement. > > During LTO the "flag" may be renamed to "flag.lto_priv.1" but the asm > > statement refers to the old name. Anything that would prevent renaming > > would also disable ipa-reference. > > > > You could do something like this: > > > > int flag; > > > > __attribute__ ((used)) > > void asm_set(void) {flag = 1;} > > > > clobber_and_set (void) { > > asm("call asm_set" ::: "memory"); > > } > > Got it, thanks. This works and fails without the changes and passes with them. > > >> An extra IL walk seems excessive for this, I wonder if we (should) > >> have recorded such fact > >> elsewhere, like in some other IPA summary? > > > > I suppose best fit is fnsummary which holds all kinds of stuff useful > > for IPA passes and is always computed. > > > Is it okay to move pass_ipa_free_fn_summary to be after pass_ipa_reference? > I think this is necessary in order to access the summaries in that pass. > Would a new 1-bit member of ipa_fn_summary called asm_memory_clobber be > acceptable here?
Yes, that looks good to me Honza >
