[clang] [clang] Fix invalid CodeGen for atomic fetch min/max on floating point values (PR #162011)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-codegen

Author: Sirui Mu (Lancern)

<details>
<summary>Changes</summary>

CodeGen for the builtin function `__atomic_min_fetch` and `__atomic_max_fetch` would emit an `atomicrmw` instruction followed by an `icmp` instruction on integral arguments. However, for floating point arguments, CodeGen keeps emitting `icmp` instruction instead of `fcmp` instruction.

This patch fixes the issue.

---
Full diff: https://github.com/llvm/llvm-project/pull/162011.diff


2 Files Affected:

- (modified) clang/lib/CodeGen/CGAtomic.cpp (+13-10) 
- (modified) clang/test/CodeGen/Atomics.c (+17) 


``````````diff
diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index 4a3446abcc78f..0a6419476c338 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -503,24 +503,27 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction &CGF, AtomicExpr *E,
 /// 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,
+                                         AtomicExpr::AtomicOp Op, bool IsInt,
+                                         bool IsSigned, llvm::Value *OldVal,
                                          llvm::Value *RHS) {
-  llvm::CmpInst::Predicate Pred;
+  llvm::CmpInst::Predicate ICmpPred;
+  llvm::FCmpInst::Predicate FCmpPred;
   switch (Op) {
   default:
     llvm_unreachable("Unexpected min/max operation");
   case AtomicExpr::AO__atomic_max_fetch:
   case AtomicExpr::AO__scoped_atomic_max_fetch:
-    Pred = IsSigned ? llvm::CmpInst::ICMP_SGT : llvm::CmpInst::ICMP_UGT;
+    ICmpPred = IsSigned ? llvm::CmpInst::ICMP_SGT : llvm::CmpInst::ICMP_UGT;
+    FCmpPred = llvm::FCmpInst::FCMP_UGT;
     break;
   case AtomicExpr::AO__atomic_min_fetch:
   case AtomicExpr::AO__scoped_atomic_min_fetch:
-    Pred = IsSigned ? llvm::CmpInst::ICMP_SLT : llvm::CmpInst::ICMP_ULT;
+    ICmpPred = IsSigned ? llvm::CmpInst::ICMP_SLT : llvm::CmpInst::ICMP_ULT;
+    FCmpPred = llvm::FCmpInst::FCMP_ULT;
     break;
   }
-  llvm::Value *Cmp = Builder.CreateICmp(Pred, OldVal, RHS, "tst");
+  llvm::Value *Cmp = IsInt ? Builder.CreateICmp(ICmpPred, OldVal, RHS, "tst")
+                           : Builder.CreateFCmp(FCmpPred, OldVal, RHS, "tst");
   return Builder.CreateSelect(Cmp, OldVal, RHS, "newval");
 }
 
@@ -760,9 +763,9 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
   // determine the value which was written.
   llvm::Value *Result = RMWI;
   if (PostOpMinMax)
-    Result = EmitPostAtomicMinMax(CGF.Builder, E->getOp(),
-                                  E->getValueType()->isSignedIntegerType(),
-                                  RMWI, LoadVal1);
+    Result = EmitPostAtomicMinMax(
+        CGF.Builder, E->getOp(), E->getValueType()->isIntegerType(),
+        E->getValueType()->isSignedIntegerType(), RMWI, LoadVal1);
   else if (PostOp)
     Result = CGF.Builder.CreateBinOp((llvm::Instruction::BinaryOps)PostOp, RMWI,
                                      LoadVal1);
diff --git a/clang/test/CodeGen/Atomics.c b/clang/test/CodeGen/Atomics.c
index c44f45efacab0..dd5936cf48b46 100644
--- a/clang/test/CodeGen/Atomics.c
+++ b/clang/test/CodeGen/Atomics.c
@@ -12,6 +12,7 @@ signed long long sll;
 unsigned long long ull;
 __int128 s128;
 unsigned  __int128 u128;
+float f;
 
 void test_op_ignore (void) // CHECK-LABEL: define{{.*}} void @test_op_ignore
 {
@@ -318,3 +319,19 @@ void test_atomic(void) {
   ui = __atomic_fetch_max(&ui, 5, __ATOMIC_ACQUIRE); // CHECK: atomicrmw umax {{.*}} acquire, align 4
   si = __atomic_fetch_max(&si, 5, __ATOMIC_RELEASE); // CHECK: atomicrmw max {{.*}} release, align 4
 }
+
+void test_atomic_minmax_fp(void) {
+  f = __atomic_fetch_min(&f, 5.0f, __ATOMIC_RELAXED);
+  // CHECK: atomicrmw fmin {{.*}} monotonic, align 4
+
+  f = __atomic_fetch_max(&f, 5.0, __ATOMIC_RELAXED);
+  // CHECK: atomicrmw fmax {{.*}} monotonic, align 4
+
+  f = __atomic_min_fetch(&f, 5.0, __ATOMIC_RELAXED);
+  // CHECK:      atomicrmw fmin {{.*}} monotonic, align 4
+  // CHECK-NEXT: fcmp ult float {{.*}}, {{.*}}
+
+  f = __atomic_max_fetch(&f, 5.0, __ATOMIC_RELAXED);
+  // CHECK:      atomicrmw fmax {{.*}} monotonic, align 4
+  // CHECK-NEXT: fcmp ugt float {{.*}}, {{.*}}
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/162011