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

[Ellis Hoag via Phabricator via llvm-commits]

ellis added a comment.

It would be good to test that atomics are not used for targets that don't support them. I think it would look similar to `llvm/test/Instrumentation/InstrProfiling/atomic-updates.ll`.

Another option would be to throw an error if the user attempts to pass one of the atomic flags (e.g., `-instrprof-atomic-counter-update-all`, `-atomic-first-counter`, ...) on targets that don't support atomics. Or maybe we should at least report a warning? I'm not sure what the precedent is.

I'm also adding other reviewers that might have more context.



================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:454
+  if (targetSupportsAtomic()) {
+    if (DoCounterPromotion.getNumOccurrences() > 0)
+      return DoCounterPromotion;
----------------
I think this can be refactored to 

```
if (!targetSupportsAtomic())
  return false;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152328