[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

Given that these are overloaded builtins, why are there separate builtins for `min` and `umin`?

If this is a Clang extension, it needs to be documented in our extensions documentation.

The documentation should describe the exact ordering semantics, to the extent we want to guarantee them.  For example, does `__atomic_fetch_max` order this operation with later operations even if the new value is in fact less than the current value of the variable, or does it use some treatment more like the failure case of a compare-exchange?

The documentation should describe the set of allowed types.  If the builtins work on pointer types, the documentation should describe the semantics of the pointer comparison; for example, is it undefined behavior if an ordinary pointer comparison would be?  Also, your test case should actually check the well-formedness conditions more completely, e.g. verifying that the value type is convertible to the atomic type.


Repository:
  rC Clang

https://reviews.llvm.org/D46386