[InstCombine] Extend "idempotent" atomicrmw optimizations to floating point

An idempotent atomicrmw is one that does not change memory in the process of execution.  We have already added handling for the various integer operations; this patch extends the same handling to floating point operations which were recently added to IR.

Note: At the moment, we canonicalize idempotent fsub to fadd when ordering requirements prevent us from using a load.  As discussed in the review, I will be replacing this with canonicalizing both floating point ops to integer ops in the near future.

Differential Revision: https://reviews.llvm.org/D58251

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@355210 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp b/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
index b857741e8402f7754cae1c8c1f90e1768b8ca6a3..d3a7d32ec7588cff66c079ec14ee5cb5a04b675f 100644 (file)
--- a/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
+++ b/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
@@ -21,9 +21,18 @@ namespace {
 /// TODO: Common w/ the version in AtomicExpandPass, and change the term used.
 /// Idemptotent is confusing in this context.
 bool isIdempotentRMW(AtomicRMWInst& RMWI) {
+  if (auto CF = dyn_cast<ConstantFP>(RMWI.getValOperand()))
+    switch(RMWI.getOperation()) {
+    case AtomicRMWInst::FAdd: // -0.0
+      return CF->isZero() && CF->isNegative();
+    case AtomicRMWInst::FSub: // +0.0
+      return CF->isZero() && !CF->isNegative();
+    default:
+      return false;
+    };
+  
   auto C = dyn_cast<ConstantInt>(RMWI.getValOperand());
   if(!C)
-    // TODO: Handle fadd, fsub?
     return false;
 
   switch(RMWI.getOperation()) {
@@ -116,12 +125,18 @@ Instruction *InstCombiner::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
 
   // We chose to canonicalize all idempotent operations to an single
   // operation code and constant.  This makes it easier for the rest of the
-  // optimizer to match easily.  The choice of or w/zero is arbitrary.
+  // optimizer to match easily.  The choices of or w/0 and fadd w/-0.0 are
+  // arbitrary. 
   if (RMWI.getType()->isIntegerTy() &&
       RMWI.getOperation() != AtomicRMWInst::Or) {
     RMWI.setOperation(AtomicRMWInst::Or);
     RMWI.setOperand(1, ConstantInt::get(RMWI.getType(), 0));
     return &RMWI;
+  } else if (RMWI.getType()->isFloatingPointTy() &&
+             RMWI.getOperation() != AtomicRMWInst::FAdd) {
+    RMWI.setOperation(AtomicRMWInst::FAdd);
+    RMWI.setOperand(1, ConstantFP::getNegativeZero(RMWI.getType()));
+    return &RMWI;
   }
 
   // Check if the required ordering is compatible with an atomic load.

diff --git a/test/Transforms/InstCombine/atomicrmw.ll b/test/Transforms/InstCombine/atomicrmw.ll
index c37a626658b3506afcec908f86a9416ee53d8d38..78c8ce1085a2f7e9a8d04ff5824d263a893fff9c 100644 (file)
--- a/test/Transforms/InstCombine/atomicrmw.ll
+++ b/test/Transforms/InstCombine/atomicrmw.ll
@@ -67,6 +67,29 @@ define i8 @atomic_max_smin_char(i8* %addr) {
   ret i8 %res
 }
 
+; CHECK-LABEL: atomic_fsub
+; CHECK-NEXT: %res = load atomic float, float* %addr monotonic, align 4
+; CHECK-NEXT: ret float %res
+define float @atomic_fsub_zero(float* %addr) {
+  %res = atomicrmw fsub float* %addr, float 0.0 monotonic
+  ret float %res
+}
+
+; CHECK-LABEL: atomic_fadd
+; CHECK-NEXT: %res = load atomic float, float* %addr monotonic, align 4
+; CHECK-NEXT: ret float %res
+define float @atomic_fadd_zero(float* %addr) {
+  %res = atomicrmw fadd float* %addr, float -0.0 monotonic
+  ret float %res
+}
+
+; CHECK-LABEL: atomic_fsub_canon
+; CHECK-NEXT: %res = atomicrmw fadd float* %addr, float -0.000000e+00 release
+; CHECK-NEXT: ret float %res
+define float @atomic_fsub_canon(float* %addr) {
+  %res = atomicrmw fsub float* %addr, float 0.0 release
+  ret float %res
+}
 
 ; Can't replace a volatile w/a load; this would eliminate a volatile store.
 ; CHECK-LABEL: atomic_sub_zero_volatile