[LowerAtomic] Lower fadd and fsub atomicrmw instructions

[jberdine]

Summary

FAdd and FSub have recently (r351850) been added as atomicrmw
operations. This diff adds lowering cases for them to the LowerAtomic
transform.

[arsenm]

Needs tests. Also I'm about 90% sure I added this already?

[arsenm]

Apparently we have both lib/CodeGEn/AtomicExpandPass.cpp, and lib/Transforms/Scalar/LowerAtomic.cpp. It's unclear to me what the difference is, or why both exist

[jberdine]

My understanding (which shouldn't be taken to imply much) is that lib/Transforms/Scalar/LowerAtomic.cpp is for contexts where atomic semantics is known to be unnecessary due to e.g. executing only in a sequential context.

[arsenm]

This still needs tests, but it seems this duplicate pass thing could use some work. Both passes have essentially the same switch to turn the atomic into the equivalent non-atomic. It seems to be me like AtomicExpandPass is a superset of LowerAtomic, except AtomicExpand seems to ignore fences

[jberdine]

add tests

[jberdine]

I see what you mean about the redundancy between the LowerAtomic and AtomicExpand passes.

Are the added tests what you had in mind?

[arsenm]

LGTM

[jberdine]

Thanks! Note that I don't have write access, so could I ask you to commit this?

[compnerd]

SVN r361512

lib/Transforms/Scalar/LowerAtomic.cpp

static bool LowerAtomicRMWInst(AtomicRMWInst *RMWI) {
   case AtomicRMWInst::UMax:
     Res = Builder.CreateSelect(Builder.CreateICmpULT(Orig, Val),
                                Val, Orig);
     break;
   case AtomicRMWInst::UMin:
     Res = Builder.CreateSelect(Builder.CreateICmpULT(Orig, Val),
                                Orig, Val);
     break;
+  case AtomicRMWInst::FAdd:
+    Res = Builder.CreateFAdd(Orig, Val);
+    break;
+  case AtomicRMWInst::FSub:
+    Res = Builder.CreateFSub(Orig, Val);
+    break;
   }
   Builder.CreateStore(Res, Ptr);
   RMWI->replaceAllUsesWith(Orig);
   RMWI->eraseFromParent();
   return true;
 }
 
 static bool LowerFenceInst(FenceInst *FI) {

test/Transforms/LowerAtomic/atomic-load.ll

; CHECK-LABEL: @min(
   %j = atomicrmw min i8* %i, i8 42 monotonic
 ; CHECK: [[INST:%[a-z0-9]+]] = load
 ; CHECK-NEXT: icmp
 ; CHECK-NEXT: select
 ; CHECK-NEXT: store
   ret i8 %j
 ; CHECK: ret i8 [[INST]]
 }
+
+define float @fadd() {
+; CHECK-LABEL: @fadd(
+  %i = alloca float
+  %j = atomicrmw fadd float* %i, float 42.0 monotonic
+; CHECK: [[INST:%[a-z0-9]+]] = load
+; CHECK-NEXT: fadd
+; CHECK-NEXT: store
+  ret float %j
+; CHECK: ret float [[INST]]
+}
+
+define float @fsub() {
+; CHECK-LABEL: @fsub(
+  %i = alloca float
+  %j = atomicrmw fsub float* %i, float 42.0 monotonic
+; CHECK: [[INST:%[a-z0-9]+]] = load
+; CHECK-NEXT: fsub
+; CHECK-NEXT: store
+  ret float %j
+; CHECK: ret float [[INST]]
+}