[PATCH] D29703: [XRay] [compiler-rt] Allow logging the first argument of a function call.

[Martin Pelikán via Phabricator via llvm-commits]

pelikan added a comment.

I think this should land ASAP so we can get some testing.  Things can be polished later.



================
Comment at: lib/xray/xray_interface.cc:217
+{
+  XRayArgLogger.store(Handler, std::memory_order_relaxed);
+}
----------------
dberris wrote:
> pelikan wrote:
> > dberris wrote:
> > > I think it would be better to make this store a release, for a stronger guarantee. 
> > Stronger guarantee of what?  There's nothing we're synchronizing *with*, we only care about atomicity (which this provides).  When I see a release/consume I automatically look for an acquire, and in this case there isn't one.
> In x86-64 the acquire is in the assembly (spelled `movq`) because of the stronger memory model. That would change in weaker memory models, and since this is in a cross-platform part of the code, we have to be a bit more careful about this.
Why do we care which order do the store and load happen?  I can't think of a scenario where I'm enabling arg1 logging on one CPU and worrying about what happens on other CPUs at the same time with respect to me just enabling it.  How is eventual consistency not enough?

Similarly, due to the strong memory model, you can look at the MOV as a relaxed operation that just got unnecessarily expensive because of Intel ;-)


https://reviews.llvm.org/D29703