[Bug ipa/80728] IPA-reference suppresses compiler memory barrier
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80728
--- Comment #9 from Andrew Pinski ---
I am not 100% sure we should not transform this code. The way signals are
defined that only volatile/atomic variables can be done via signals.
This works:
```
static _Atomic int i;
static _Atomic int b;
void sighandler(void)
{
__atomic_store_n (&b, 1, __ATOMIC_RELAXED);
__atomic_store_n (&i, 1, __ATOMIC_RELAXED);
}
__attribute__((noinline))
static int x(void)
{
asm volatile("":::"memory");
return __atomic_load_n(&b, __ATOMIC_RELAXED);
}
int f(void)
{
__atomic_store_n (&b, 0, __ATOMIC_RELAXED);
return x() ? __atomic_load_n(&i, __ATOMIC_RELAXED) : 0;
}
```
So does marking b/i as volatile.
Marking the variable i with the attribute used also works. as that says the
variable is used outside of what the compiler knows and in this case x only has
an inline-asm which clobbers memory.
[Bug ipa/80728] IPA-reference suppresses compiler memory barrier
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80728 --- Comment #8 from Alexander Monakov --- Honza: ping :). ipa-pure-const might be a better place to mark functions with compiler memory barriers, as it already computes and propagates "nonfreeing" property (thus computing "nonbarrier" in ipa-pure-const would allow to use it in implementation of nonbarrier_call_p). OTOH ipa-pure-const is optional, so with -fno-ipa-pure-const, ipa-reference would see most functions as compiler memory barriers. Although it's possible to rescan bodies of those functions that ipa-pure-const did not scan, that seems undesirable.
[Bug ipa/80728] IPA-reference suppresses compiler memory barrier
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80728 --- Comment #7 from rguenther at suse dot de --- On May 15, 2017 4:43:04 PM GMT+02:00, "amonakov at gcc dot gnu.org" wrote: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80728 > >--- Comment #6 from Alexander Monakov --- >I think a possible approach is to add a new cgraph_node flag (or a >multi-bit >field, if we want to track presence of acquire/release/seq-cst compiler >barriers separately), handle asms and atomics specially in >cgraphbuild.c to set >that flag, and finally use the new flag in ipa-reference.c? Yes. Something like that. Let's see if Honza has any comments.
[Bug ipa/80728] IPA-reference suppresses compiler memory barrier
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80728 --- Comment #6 from Alexander Monakov --- I think a possible approach is to add a new cgraph_node flag (or a multi-bit field, if we want to track presence of acquire/release/seq-cst compiler barriers separately), handle asms and atomics specially in cgraphbuild.c to set that flag, and finally use the new flag in ipa-reference.c?
[Bug ipa/80728] IPA-reference suppresses compiler memory barrier
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80728 --- Comment #5 from rguenther at suse dot de --- On Mon, 15 May 2017, amonakov at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80728 > > --- Comment #4 from Alexander Monakov --- > ipa-reference.c has: > > /* Set of all interesting module statics. A bit is set for every module >static we are considering. This is added to the local info when asm >code is found that clobbers all memory. */ > static bitmap all_module_statics; > > > but I don't see where the code implementing the last statement in the comment > is. > I only see how indirect calls are handled (and I think barriers due to atomics > and asms can be handled in a similar fashion). Yeah, the comment refers to the old implementation (which likely was fine). The new implementation simply uses the ipa_ref lists instead of walking stmts.
[Bug ipa/80728] IPA-reference suppresses compiler memory barrier
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80728 --- Comment #4 from Alexander Monakov --- ipa-reference.c has: /* Set of all interesting module statics. A bit is set for every module static we are considering. This is added to the local info when asm code is found that clobbers all memory. */ static bitmap all_module_statics; but I don't see where the code implementing the last statement in the comment is. I only see how indirect calls are handled (and I think barriers due to atomics and asms can be handled in a similar fashion).
[Bug ipa/80728] IPA-reference suppresses compiler memory barrier
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80728
--- Comment #3 from rguenther at suse dot de ---
On Mon, 15 May 2017, amonakov at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80728
>
> --- Comment #2 from Alexander Monakov ---
> Nowadays C has atomics and fences in the language standard, so it doesn't
> matter if x() had
>
> asm volatile("":::"memory");
>
> or
>
> __atomic_{signal,thread}_fence(__ATOMIC_ACQ_REL);
>
> or
>
> return __atomic_load_n(&b, __ATOMIC_SEQ_CST);
>
> In all three cases the compiler needs to place a memory barrier internally,
> and
> in the latter two it's impossible to argue that the source code is missing a
> clobber.
>
> I think it's correct that "memory" clobber cannot touch non-addressable auto
> vars, but extending that to static variables seems wrong. I'm not aware of any
> instance (apart from this bug) where gcc does that.
I think the IPA reference bug came across in another PR already, given
that IPA reference lacks references to unknown targets (it only records
direct decl references) I can't see how to easily fix this...
Well. With a flag obviously, but that must be already there somehow
given pointer dereferences are not recorded either.
[Bug ipa/80728] IPA-reference suppresses compiler memory barrier
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80728
--- Comment #2 from Alexander Monakov ---
Nowadays C has atomics and fences in the language standard, so it doesn't
matter if x() had
asm volatile("":::"memory");
or
__atomic_{signal,thread}_fence(__ATOMIC_ACQ_REL);
or
return __atomic_load_n(&b, __ATOMIC_SEQ_CST);
In all three cases the compiler needs to place a memory barrier internally, and
in the latter two it's impossible to argue that the source code is missing a
clobber.
I think it's correct that "memory" clobber cannot touch non-addressable auto
vars, but extending that to static variables seems wrong. I'm not aware of any
instance (apart from this bug) where gcc does that.
[Bug ipa/80728] IPA-reference suppresses compiler memory barrier
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80728 Richard Biener changed: What|Removed |Added CC||hubicka at gcc dot gnu.org, ||rguenth at gcc dot gnu.org --- Comment #1 from Richard Biener --- x/3 (x) @0x7f32cb27a170 Type: function definition analyzed Visibility: prevailing_def_ironly References: b/1 (read) Referring: Availability: local First run: 0 Function flags: body local Called by: f/4 (1.00 per call) Calls: so IPA references are only listing explicit references and referring/references has no way to catch the "escape" site. Honza? Note that in other PRs we explicitely said that "locals" (which includes static globals) need explicit clobbering. That is, automatic vars that do not have their address taken are not considered accessible by a "memory" clobber either.
