[PATCH] D152328: InstrProf - don't emit 64 bit atomic operations on 32 bit platforms

[Sean Mollet via Phabricator via llvm-commits]

SeanMollet added a comment.

In D152328#4435805 <https://reviews.llvm.org/D152328#4435805>, @davidxl wrote:

> Regarding improving performance, it is more about improving the optimized build (profile-use) performance instead of the performance of the instrumented build. This is because the atomic update allows more precise counter update in multi-threaded programs.

I agree that it's about improving the profiler performance. The full software approach would improve precision while profling multi-threaded programs at the cost of substantially slowing down the resulting program and potentially impacting the timing.

In general, the targets this change would apply to are older or embedded, slow and usually single core systems. In my particular case, I'm working on a 600 Mhz embedded Mips system. It's already slow and the code isn't very threaded. Given the choice, I'm perfectly happy to give up some accuracy for faster profiling.

If I was building something for a server or modern laptop/desktop/mobile phone, I'd make the opposite choice. But, they all support hardware atomics, so it's moot.



================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:456
 
 bool InstrProfiling::isCounterPromotionEnabled() const {
+  if (!targetSupportsAtomic())
----------------
davidxl wrote:
> why disabling counter promotion? It is a different from atomic counter update.
Because it also sometimes lowers to an atomic counter update. As an alternative, it could be re-worked to not do that, but then it doesn't provide meaningful benefit.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152328/new/

https://reviews.llvm.org/D152328