t.p.northover created this revision.
t.p.northover added reviewers: delena, yaxunl.
Herald added subscribers: jfb, mcrosier.

We seem to have been gradually growing support for atomic min/max operations 
(exposing longstanding IR atomicrmw instructions). But until now there have 
been gaps in the expected intrinsics. This adds support for the C11-style 
intrinsics (i.e. taking _Atomic, rather than individually blessed by C11 
standard), and the variants that return the new value instead of the original 
one.

That way, people won't be misled by trying one form and it not working, and the 
front-end is more friendly to people using _Atomic types, as we recommend.


Repository:
  rC Clang

https://reviews.llvm.org/D55562

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/atomic-ops-libcall.c
  clang/test/CodeGen/atomic-ops.c
  clang/test/Sema/atomic-ops.c

Index: clang/test/Sema/atomic-ops.c
===================================================================
--- clang/test/Sema/atomic-ops.c
+++ clang/test/Sema/atomic-ops.c
@@ -354,6 +354,20 @@
   (void)__c11_atomic_fetch_xor(Ap, val, memory_order_acq_rel);
   (void)__c11_atomic_fetch_xor(Ap, val, memory_order_seq_cst);
 
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_relaxed);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_acquire);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_consume);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_release);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_acq_rel);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_seq_cst);
+
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_relaxed);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_acquire);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_consume);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_release);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_acq_rel);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_seq_cst);
+
   (void)__c11_atomic_exchange(Ap, val, memory_order_relaxed);
   (void)__c11_atomic_exchange(Ap, val, memory_order_acquire);
   (void)__c11_atomic_exchange(Ap, val, memory_order_consume);
@@ -501,6 +515,20 @@
   (void)__atomic_nand_fetch(p, val, memory_order_acq_rel);
   (void)__atomic_nand_fetch(p, val, memory_order_seq_cst);
 
+  (void)__atomic_max_fetch(p, val, memory_order_relaxed);
+  (void)__atomic_max_fetch(p, val, memory_order_acquire);
+  (void)__atomic_max_fetch(p, val, memory_order_consume);
+  (void)__atomic_max_fetch(p, val, memory_order_release);
+  (void)__atomic_max_fetch(p, val, memory_order_acq_rel);
+  (void)__atomic_max_fetch(p, val, memory_order_seq_cst);
+
+  (void)__atomic_min_fetch(p, val, memory_order_relaxed);
+  (void)__atomic_min_fetch(p, val, memory_order_acquire);
+  (void)__atomic_min_fetch(p, val, memory_order_consume);
+  (void)__atomic_min_fetch(p, val, memory_order_release);
+  (void)__atomic_min_fetch(p, val, memory_order_acq_rel);
+  (void)__atomic_min_fetch(p, val, memory_order_seq_cst);
+
   (void)__atomic_exchange_n(p, val, memory_order_relaxed);
   (void)__atomic_exchange_n(p, val, memory_order_acquire);
   (void)__atomic_exchange_n(p, val, memory_order_consume);
Index: clang/test/CodeGen/atomic-ops.c
===================================================================
--- clang/test/CodeGen/atomic-ops.c
+++ clang/test/CodeGen/atomic-ops.c
@@ -661,4 +661,46 @@
   __atomic_compare_exchange(&aligned_a, &aligned_b, &aligned_c, 1, memory_order_seq_cst, memory_order_seq_cst);
 }
 
+void test_c11_minmax(_Atomic(int) *si, _Atomic(unsigned) *ui) {
+  // CHECK-LABEL: @test_c11_minmax
+
+  // CHECK: atomicrmw max
+  *si = __c11_atomic_fetch_max(si, 42, memory_order_acquire);
+  // CHECK: atomicrmw min
+  *si = __c11_atomic_fetch_min(si, 42, memory_order_acquire);
+  // CHECK: atomicrmw umax
+  *ui = __c11_atomic_fetch_max(ui, 42, memory_order_acquire);
+  // CHECK: atomicrmw umin
+  *ui = __c11_atomic_fetch_min(ui, 42, memory_order_acquire);
+}
+
+void test_minmax_postop(int *si, unsigned *ui) {
+  int val = 42;
+  // CHECK-LABEL: @test_minmax_postop
+
+  // CHECK: [[OLD:%.*]] = atomicrmw max i32* [[PTR:%.*]], i32 [[RHS:%.*]] release
+  // CHECK: [[TST:%.*]] = icmp sgt i32 [[OLD]], [[RHS]]
+  // CHECK: [[NEW:%.*]] = select i1 [[TST]], i32 [[OLD]], i32 [[RHS]]
+  // CHECK: store i32 [[NEW]], i32*
+  *si = __atomic_max_fetch(si, 42, memory_order_release);
+
+  // CHECK: [[OLD:%.*]] = atomicrmw min i32* [[PTR:%.*]], i32 [[RHS:%.*]] release
+  // CHECK: [[TST:%.*]] = icmp slt i32 [[OLD]], [[RHS]]
+  // CHECK: [[NEW:%.*]] = select i1 [[TST]], i32 [[OLD]], i32 [[RHS]]
+  // CHECK: store i32 [[NEW]], i32*
+  *si = __atomic_min_fetch(si, 42, memory_order_release);
+  
+  // CHECK: [[OLD:%.*]] = atomicrmw umax i32* [[PTR:%.*]], i32 [[RHS:%.*]] release
+  // CHECK: [[TST:%.*]] = icmp ugt i32 [[OLD]], [[RHS]]
+  // CHECK: [[NEW:%.*]] = select i1 [[TST]], i32 [[OLD]], i32 [[RHS]]
+  // CHECK: store i32 [[NEW]], i32*
+  *ui = __atomic_max_fetch(ui, 42, memory_order_release);
+
+  // CHECK: [[OLD:%.*]] = atomicrmw umin i32* [[PTR:%.*]], i32 [[RHS:%.*]] release
+  // CHECK: [[TST:%.*]] = icmp ult i32 [[OLD]], [[RHS]]
+  // CHECK: [[NEW:%.*]] = select i1 [[TST]], i32 [[OLD]], i32 [[RHS]]
+  // CHECK: store i32 [[NEW]], i32*
+  *ui = __atomic_min_fetch(ui, 42, memory_order_release);
+}
+
 #endif
Index: clang/test/CodeGen/atomic-ops-libcall.c
===================================================================
--- clang/test/CodeGen/atomic-ops-libcall.c
+++ clang/test/CodeGen/atomic-ops-libcall.c
@@ -111,10 +111,38 @@
 }
 
 int test_atomic_nand_fetch(int *p) {
-  // CHECK: test_atomic_nand_fetch
+  // CHECK-LABEL: test_atomic_nand_fetch
   // CHECK: [[CALL:%[^ ]*]] = tail call i32 @__atomic_fetch_nand_4(i8* {{%[0-9]+}}, i32 55, i32 5)
   // FIXME: We should not be checking optimized IR. It changes independently of clang.
   // FIXME-CHECK: [[AND:%[^ ]*]] = and i32 [[CALL]], 55
   // FIXME-CHECK: {{%[^ ]*}} = xor i32 [[AND]], -1
   return __atomic_nand_fetch(p, 55, memory_order_seq_cst);
 }
+
+int test_atomic_fetch_max(int *p) {
+  // CHECK-LABEL: test_atomic_fetch_max
+  // CHECK: {{%[^ ]*}} = tail call i32 @__atomic_fetch_max_4(i8* {{%[0-9]+}}, i32 55, i32 5)
+  return __atomic_fetch_max(p, 55, memory_order_seq_cst);
+}
+
+int test_atomic_fetch_min(int *p) {
+  // CHECK: test_atomic_fetch_min
+  // CHECK: {{%[^ ]*}} = tail call i32 @__atomic_fetch_min_4(i8* {{%[0-9]+}}, i32 55, i32 5)
+  return __atomic_fetch_min(p, 55, memory_order_seq_cst);
+}
+
+int test_atomic_max_fetch(int *p) {
+  // CHECK-LABEL: test_atomic_max_fetch
+  // CHECK: [[CALL:%[^ ]*]] = tail call i32 @__atomic_fetch_max_4(i8* {{%[0-9]+}}, i32 55, i32 5)
+  // CHECK: [[TST:%[^ ]*]] = icmp sgt i32 [[CALL]], 55
+  // CHECK: {{%[^ ]*}} = select i1 [[TST]], i32 [[CALL]], i32 55
+  return __atomic_max_fetch(p, 55, memory_order_seq_cst);
+}
+
+int test_atomic_min_fetch(int *p) {
+  // CHECK-LABEL: test_atomic_min_fetch
+  // CHECK: [[CALL:%[^ ]*]] = tail call i32 @__atomic_fetch_min_4(i8* {{%[0-9]+}}, i32 55, i32 5)
+  // CHECK: [[TST:%.*]] = icmp slt i32 [[CALL]], 55
+  // CHECK: {{%[^ ]*}} = select i1 [[TST]], i32 [[CALL]], i32 55
+  return __atomic_min_fetch(p, 55, memory_order_seq_cst);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4333,13 +4333,13 @@
       && sizeof(NumVals)/sizeof(NumVals[0]) == NumForm,
       "need to update code for modified forms");
   static_assert(AtomicExpr::AO__c11_atomic_init == 0 &&
-                    AtomicExpr::AO__c11_atomic_fetch_xor + 1 ==
+                    AtomicExpr::AO__c11_atomic_fetch_min + 1 ==
                         AtomicExpr::AO__atomic_load,
                 "need to update code for modified C11 atomics");
   bool IsOpenCL = Op >= AtomicExpr::AO__opencl_atomic_init &&
                   Op <= AtomicExpr::AO__opencl_atomic_fetch_max;
   bool IsC11 = (Op >= AtomicExpr::AO__c11_atomic_init &&
-               Op <= AtomicExpr::AO__c11_atomic_fetch_xor) ||
+               Op <= AtomicExpr::AO__c11_atomic_fetch_min) ||
                IsOpenCL;
   bool IsN = Op == AtomicExpr::AO__atomic_load_n ||
              Op == AtomicExpr::AO__atomic_store_n ||
@@ -4400,6 +4400,10 @@
     Form = Arithmetic;
     break;
 
+  case AtomicExpr::AO__c11_atomic_fetch_min:
+  case AtomicExpr::AO__c11_atomic_fetch_max:
+  case AtomicExpr::AO__atomic_min_fetch:
+  case AtomicExpr::AO__atomic_max_fetch:
   case AtomicExpr::AO__atomic_fetch_min:
   case AtomicExpr::AO__atomic_fetch_max:
     IsMinMax = true;
Index: clang/lib/CodeGen/CGAtomic.cpp
===================================================================
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/lib/CodeGen/CGAtomic.cpp
@@ -490,6 +490,28 @@
   CGF.Builder.SetInsertPoint(ContBB);
 }
 
+/// Duplicate the atomic min/max operation in conventional IR for the builtin
+/// variants that return the new rather than the original value.
+static llvm::Value *EmitPostAtomicMinMax(CGBuilderTy &Builder,
+                                         AtomicExpr::AtomicOp Op,
+                                         bool IsSigned,
+                                         llvm::Value *OldVal,
+                                         llvm::Value *RHS) {
+  llvm::CmpInst::Predicate Pred;
+  switch (Op) {
+  default:
+    llvm_unreachable("Unexpected min/max operation");
+  case AtomicExpr::AO__atomic_max_fetch:
+    Pred = IsSigned ? llvm::CmpInst::ICMP_SGT : llvm::CmpInst::ICMP_UGT;
+    break;
+  case AtomicExpr::AO__atomic_min_fetch:
+    Pred = IsSigned ? llvm::CmpInst::ICMP_SLT : llvm::CmpInst::ICMP_ULT;
+    break;
+  }
+  llvm::Value *Cmp = Builder.CreateICmp(Pred, OldVal, RHS, "tst");
+  return Builder.CreateSelect(Cmp, OldVal, RHS, "newval");
+}
+
 static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
                          Address Ptr, Address Val1, Address Val2,
                          llvm::Value *IsWeak, llvm::Value *FailureOrder,
@@ -497,6 +519,7 @@
                          llvm::SyncScope::ID Scope) {
   llvm::AtomicRMWInst::BinOp Op = llvm::AtomicRMWInst::Add;
   llvm::Instruction::BinaryOps PostOp = (llvm::Instruction::BinaryOps)0;
+  bool PostOpMinMax = false;
 
   switch (E->getOp()) {
   case AtomicExpr::AO__c11_atomic_init:
@@ -590,12 +613,20 @@
     Op = llvm::AtomicRMWInst::Sub;
     break;
 
+  case AtomicExpr::AO__atomic_min_fetch:
+    PostOpMinMax = true;
+    LLVM_FALLTHROUGH;
+  case AtomicExpr::AO__c11_atomic_fetch_min:
   case AtomicExpr::AO__opencl_atomic_fetch_min:
   case AtomicExpr::AO__atomic_fetch_min:
     Op = E->getValueType()->isSignedIntegerType() ? llvm::AtomicRMWInst::Min
                                                   : llvm::AtomicRMWInst::UMin;
     break;
 
+  case AtomicExpr::AO__atomic_max_fetch:
+    PostOpMinMax = true;
+    LLVM_FALLTHROUGH;
+  case AtomicExpr::AO__c11_atomic_fetch_max:
   case AtomicExpr::AO__opencl_atomic_fetch_max:
   case AtomicExpr::AO__atomic_fetch_max:
     Op = E->getValueType()->isSignedIntegerType() ? llvm::AtomicRMWInst::Max
@@ -645,7 +676,11 @@
   // For __atomic_*_fetch operations, perform the operation again to
   // determine the value which was written.
   llvm::Value *Result = RMWI;
-  if (PostOp)
+  if (PostOpMinMax)
+    Result = EmitPostAtomicMinMax(CGF.Builder, E->getOp(),
+                                  E->getValueType()->isSignedIntegerType(),
+                                  RMWI, LoadVal1);
+  else if (PostOp)
     Result = CGF.Builder.CreateBinOp(PostOp, RMWI, LoadVal1);
   if (E->getOp() == AtomicExpr::AO__atomic_nand_fetch)
     Result = CGF.Builder.CreateNot(Result);
@@ -852,6 +887,8 @@
   case AtomicExpr::AO__c11_atomic_fetch_and:
   case AtomicExpr::AO__c11_atomic_fetch_or:
   case AtomicExpr::AO__c11_atomic_fetch_xor:
+  case AtomicExpr::AO__c11_atomic_fetch_max:
+  case AtomicExpr::AO__c11_atomic_fetch_min:
   case AtomicExpr::AO__opencl_atomic_fetch_and:
   case AtomicExpr::AO__opencl_atomic_fetch_or:
   case AtomicExpr::AO__opencl_atomic_fetch_xor:
@@ -865,8 +902,10 @@
   case AtomicExpr::AO__atomic_or_fetch:
   case AtomicExpr::AO__atomic_xor_fetch:
   case AtomicExpr::AO__atomic_nand_fetch:
-  case AtomicExpr::AO__atomic_fetch_min:
+  case AtomicExpr::AO__atomic_max_fetch:
+  case AtomicExpr::AO__atomic_min_fetch:
   case AtomicExpr::AO__atomic_fetch_max:
+  case AtomicExpr::AO__atomic_fetch_min:
     Val1 = EmitValToTemp(*this, E->getVal1());
     break;
   }
@@ -915,14 +954,18 @@
     case AtomicExpr::AO__opencl_atomic_fetch_min:
     case AtomicExpr::AO__opencl_atomic_fetch_max:
     case AtomicExpr::AO__atomic_fetch_xor:
+    case AtomicExpr::AO__c11_atomic_fetch_max:
+    case AtomicExpr::AO__c11_atomic_fetch_min:
     case AtomicExpr::AO__atomic_add_fetch:
     case AtomicExpr::AO__atomic_and_fetch:
     case AtomicExpr::AO__atomic_nand_fetch:
     case AtomicExpr::AO__atomic_or_fetch:
     case AtomicExpr::AO__atomic_sub_fetch:
     case AtomicExpr::AO__atomic_xor_fetch:
-    case AtomicExpr::AO__atomic_fetch_min:
     case AtomicExpr::AO__atomic_fetch_max:
+    case AtomicExpr::AO__atomic_fetch_min:
+    case AtomicExpr::AO__atomic_max_fetch:
+    case AtomicExpr::AO__atomic_min_fetch:
       // For these, only library calls for certain sizes exist.
       UseOptimizedLibcall = true;
       break;
@@ -990,6 +1033,7 @@
     QualType RetTy;
     bool HaveRetTy = false;
     llvm::Instruction::BinaryOps PostOp = (llvm::Instruction::BinaryOps)0;
+    bool PostOpMinMax = false;
     switch (E->getOp()) {
     case AtomicExpr::AO__c11_atomic_init:
     case AtomicExpr::AO__opencl_atomic_init:
@@ -1111,6 +1155,10 @@
       AddDirectArgument(*this, Args, UseOptimizedLibcall, Val1.getPointer(),
                         MemTy, E->getExprLoc(), sizeChars);
       break;
+    case AtomicExpr::AO__atomic_min_fetch:
+      PostOpMinMax = true;
+      LLVM_FALLTHROUGH;
+    case AtomicExpr::AO__c11_atomic_fetch_min:
     case AtomicExpr::AO__atomic_fetch_min:
     case AtomicExpr::AO__opencl_atomic_fetch_min:
       LibCallName = E->getValueType()->isSignedIntegerType()
@@ -1119,6 +1167,10 @@
       AddDirectArgument(*this, Args, UseOptimizedLibcall, Val1.getPointer(),
                         LoweredMemTy, E->getExprLoc(), sizeChars);
       break;
+    case AtomicExpr::AO__atomic_max_fetch:
+      PostOpMinMax = true;
+      LLVM_FALLTHROUGH;
+    case AtomicExpr::AO__c11_atomic_fetch_max:
     case AtomicExpr::AO__atomic_fetch_max:
     case AtomicExpr::AO__opencl_atomic_fetch_max:
       LibCallName = E->getValueType()->isSignedIntegerType()
@@ -1170,7 +1222,7 @@
     // PostOp is only needed for the atomic_*_fetch operations, and
     // thus is only needed for and implemented in the
     // UseOptimizedLibcall codepath.
-    assert(UseOptimizedLibcall || !PostOp);
+    assert(UseOptimizedLibcall || (!PostOp && !PostOpMinMax));
 
     RValue Res = emitAtomicLibcall(*this, LibCallName, RetTy, Args);
     // The value is returned directly from the libcall.
@@ -1181,7 +1233,12 @@
     // provided an out-param.
     if (UseOptimizedLibcall && Res.getScalarVal()) {
       llvm::Value *ResVal = Res.getScalarVal();
-      if (PostOp) {
+      if (PostOpMinMax) {
+        llvm::Value *LoadVal1 = Args[1].getRValue(*this).getScalarVal();
+        ResVal = EmitPostAtomicMinMax(Builder, E->getOp(),
+                                      E->getValueType()->isSignedIntegerType(),
+                                      ResVal, LoadVal1);
+      } else if (PostOp) {
         llvm::Value *LoadVal1 = Args[1].getRValue(*this).getScalarVal();
         ResVal = Builder.CreateBinOp(PostOp, ResVal, LoadVal1);
       }
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -4169,6 +4169,8 @@
   case AO__c11_atomic_fetch_and:
   case AO__c11_atomic_fetch_or:
   case AO__c11_atomic_fetch_xor:
+  case AO__c11_atomic_fetch_max:
+  case AO__c11_atomic_fetch_min:
   case AO__atomic_fetch_add:
   case AO__atomic_fetch_sub:
   case AO__atomic_fetch_and:
@@ -4181,6 +4183,8 @@
   case AO__atomic_or_fetch:
   case AO__atomic_xor_fetch:
   case AO__atomic_nand_fetch:
+  case AO__atomic_min_fetch:
+  case AO__atomic_max_fetch:
   case AO__atomic_fetch_min:
   case AO__atomic_fetch_max:
     return 3;
Index: clang/include/clang/Basic/Builtins.def
===================================================================
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -689,6 +689,8 @@
 ATOMIC_BUILTIN(__c11_atomic_fetch_and, "v.", "t")
 ATOMIC_BUILTIN(__c11_atomic_fetch_or, "v.", "t")
 ATOMIC_BUILTIN(__c11_atomic_fetch_xor, "v.", "t")
+ATOMIC_BUILTIN(__c11_atomic_fetch_max, "v.", "t")
+ATOMIC_BUILTIN(__c11_atomic_fetch_min, "v.", "t")
 BUILTIN(__c11_atomic_thread_fence, "vi", "n")
 BUILTIN(__c11_atomic_signal_fence, "vi", "n")
 BUILTIN(__c11_atomic_is_lock_free, "iz", "n")
@@ -713,6 +715,8 @@
 ATOMIC_BUILTIN(__atomic_and_fetch, "v.", "t")
 ATOMIC_BUILTIN(__atomic_or_fetch, "v.", "t")
 ATOMIC_BUILTIN(__atomic_xor_fetch, "v.", "t")
+ATOMIC_BUILTIN(__atomic_max_fetch, "v.", "t")
+ATOMIC_BUILTIN(__atomic_min_fetch, "v.", "t")
 ATOMIC_BUILTIN(__atomic_nand_fetch, "v.", "t")
 BUILTIN(__atomic_test_and_set, "bvD*i", "n")
 BUILTIN(__atomic_clear, "vvD*i", "n")
Index: clang/docs/LanguageExtensions.rst
===================================================================
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2141,6 +2141,8 @@
 * ``__c11_atomic_fetch_and``
 * ``__c11_atomic_fetch_or``
 * ``__c11_atomic_fetch_xor``
+* ``__c11_atomic_fetch_max``
+* ``__c11_atomic_fetch_min``
 
 The macros ``__ATOMIC_RELAXED``, ``__ATOMIC_CONSUME``, ``__ATOMIC_ACQUIRE``,
 ``__ATOMIC_RELEASE``, ``__ATOMIC_ACQ_REL``, and ``__ATOMIC_SEQ_CST`` are
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to