[llvm] r355210 - [InstCombine] Extend "idempotent" atomicrmw optimizations to floating point

[Philip Reames via llvm-commits]

Author: reames
Date: Fri Mar  1 10:00:07 2019
New Revision: 355210

URL: http://llvm.org/viewvc/llvm-project?rev=355210&view=rev
Log:
[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


Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
    llvm/trunk/test/Transforms/InstCombine/atomicrmw.ll

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp?rev=355210&r1=355209&r2=355210&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp Fri Mar  1 10:00:07 2019
@@ -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::visitAtomicRM
 
   // 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.

Modified: llvm/trunk/test/Transforms/InstCombine/atomicrmw.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/atomicrmw.ll?rev=355210&r1=355209&r2=355210&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/atomicrmw.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/atomicrmw.ll Fri Mar  1 10:00:07 2019
@@ -67,6 +67,29 @@ define i8 @atomic_max_smin_char(i8* %add
   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