[PATCH] D98605: [LibCalls] Add `nosync` to known library calls

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D98605#2625243 <https://reviews.llvm.org/D98605#2625243>, @aqjune wrote:

> For side-effecting operations, such as printf, I'm not 100% sure whether it is nosync. printf interacts with cout to properly flush buffers, which might do some interactions.

I tried to avoid all file operations, we can also avoid printf and friends.



================
Comment at: llvm/lib/Transforms/Utils/BuildLibCalls.cpp:528
+    Changed |= setNoSync(F);
     return Changed;
   case LibFunc_read:
----------------
aqjune wrote:
> C17's 7.22.3 Memory management functions has this paragraph:
> 
> 2. For purposes of determining the existence of a data race, memory allocation functions behave as though they accessed only memory locations accessible through their arguments and not other static duration storage. These functions may, however, visibly modify the storage that they allocate or deallocate. Calls to these functions that allocate or deallocate a particular region of memory shall occur in a single total order, and each such deallocation call shall synchronize with the next allocation (if any) in this order.
> 
> Should we conservatively assume that allocation/deallocation fns may synchronize with other threads?
My original purpose was to add `nosync` to malloc and free :(

I guess what this says is that if you have two threads.
T1: allocate memory, get address P, deallocate it.
T2: allocate memory, get address P

You now know T1 deallocated P.

Worst case we could derive this for call sites if the pointer P was never observed (=captured).


================
Comment at: llvm/lib/Transforms/Utils/BuildLibCalls.cpp:978
     Changed |= setDoesNotCapture(F, 3);
+    Changed |= setNoSync(F);
     return Changed;
----------------
aqjune wrote:
> This implies that if comparator fn executs any atomic operation then qsort raises UB, IIUC.
> I think it depends, it may want to safely call some complex function which may involve atomic operations (e.g. increase/decrease reference counter).
> I think it is safe to conservatively assume that qsort may sync with other threads.
right.


================
Comment at: llvm/lib/Transforms/Utils/BuildLibCalls.cpp:1100
+    Changed |= setNoSync(F);
     return Changed;
   case LibFunc_abs:
----------------
aqjune wrote:
> These functions may raise FE exceptions; would it be safe to assume that calling two ldexp, both of which setting FE exceptions, is UB?
I don't understand the question.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98605