Concurrency support for PNaCl ABI

lib/Transforms/NaCl/ResolvePNaClIntrinsics.cpp

Index: lib/Transforms/NaCl/ResolvePNaClIntrinsics.cpp
diff --git a/lib/Transforms/NaCl/ResolvePNaClIntrinsics.cpp b/lib/Transforms/NaCl/ResolvePNaClIntrinsics.cpp
index e4efeb67c3c3c24c5ca1103f960338dea0c134f6..39126f0fbe5c6b9c27dfc5b2b470cc86bb7704c0 100644
--- a/lib/Transforms/NaCl/ResolvePNaClIntrinsics.cpp
+++ b/lib/Transforms/NaCl/ResolvePNaClIntrinsics.cpp
@@ -17,10 +17,14 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/IR/Constant.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/NaCl.h"
+#include "llvm/IR/Value.h"
 #include "llvm/Pass.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/Transforms/NaCl.h"
 
 using namespace llvm;
@@ -37,14 +41,16 @@ namespace {
   private:
     // Some intrinsic calls are resolved simply by replacing the call with a
     // call to an alternative function with exactly the same type.
-    bool resolveSimpleCall(Function &F, Intrinsic::ID IntrinsicID,
-                           const char *TargetFunctionName);
+    bool resolveSimpleCall(Function &F, Intrinsic::ID IntrinsicID);
+    // All atomic calls are rewritten to a single instruction, and the
+    // call is removed.
+    void rewriteAtomicCall(Module *M, Function &F, Intrinsic::ID IntrinsicID,
+                           CallInst *Call);
   };
 }
 
 bool ResolvePNaClIntrinsics::resolveSimpleCall(Function &F,
-                                               Intrinsic::ID IntrinsicID,
-                                               const char *TargetFunctionName) {
+                                               Intrinsic::ID IntrinsicID) {
   Module *M = F.getParent();
   bool Changed = false;
   Function *IntrinsicFunction = Intrinsic::getDeclaration(M, IntrinsicID);
@@ -53,12 +59,18 @@ bool ResolvePNaClIntrinsics::resolveSimpleCall(Function &F,
     return false;
   }
 
-  // Expect to find the target function for this intrinsic already declared
-  Function *TargetFunction = M->getFunction(TargetFunctionName);
-  if (!TargetFunction) {
-    report_fatal_error(
-        std::string("Expected to find external declaration of ") +
-        TargetFunctionName);
+  Function *TargetFunction = NULL;
+  if (IntrinsicID == Intrinsic::nacl_setjmp ||

[eliben, 2013/06/26 16:20:57]

No no no no no :-)

resolveSimpleCall's comment is no longer correct
Neither is its name
You need to do special casing for setjmp/longjmp in two places
The same function behaves quite differently for different calls

Well, it's only 4 "no"s but please don't do this. Find a different way to
encapsulate the common code of walking the uses. You can have a common utility
method that does this and have both resolveSimpleCall and resolveAtomicCall use
it for slightly different putposes.

[JF, 2013/06/26 22:23:12]

Done: rewrote to use one class per type of rewrite, and one function that walks
calls.

+      IntrinsicID == Intrinsic::nacl_longjmp) {
+    // Expect to find the target function for this intrinsic already declared.
+    const char *TargetFunctionName = IntrinsicID == Intrinsic::nacl_setjmp
+        ? "setjmp" : "longjmp";
+    TargetFunction = M->getFunction(TargetFunctionName);
+    if (!TargetFunction) {
+      report_fatal_error(
+          std::string("Expected to find external declaration of ") +
+          TargetFunctionName);
+    }
   }
 
   for (Value::use_iterator UI = IntrinsicFunction->use_begin(),
@@ -77,7 +89,11 @@ bool ResolvePNaClIntrinsics::resolveSimpleCall(Function &F,
     // functions. These will be handled when the pass manager gets to those
     // functions.
     if (Call->getParent()->getParent() == &F) {
-      Call->setCalledFunction(TargetFunction);
+      if (IntrinsicID == Intrinsic::nacl_setjmp ||
+          IntrinsicID == Intrinsic::nacl_longjmp)
+        Call->setCalledFunction(TargetFunction);
+      else
+        rewriteAtomicCall(M, F, IntrinsicID, Call);
       Changed = true;
     }
   }
@@ -85,9 +101,97 @@ bool ResolvePNaClIntrinsics::resolveSimpleCall(Function &F,
   return Changed;
 }
 
+void ResolvePNaClIntrinsics::rewriteAtomicCall(Module *M, Function &F,
+                                               Intrinsic::ID IntrinsicID,
+                                               CallInst *Call) {
+  // Assume the @nacl.atomic.<size> intrinsics follow the PNaCl
+  // ABI: this should have been checked by the verifier.
+  uint64_t Op(cast<Constant>(
+      Call->getArgOperand(0))->getUniqueInteger().getLimitedValue());
+  Value *Ptr(Call->getArgOperand(1));

[Mark Seaborn, 2013/06/26 14:33:41]

Just write "Ptr = Call->getArgOperand(1)"?  Using Var(Init) is odd for a plain
old variable.

[JF, 2013/06/26 15:52:29]

Done.

+  Value *Val(Call->getArgOperand(2));
+  Value *Old(Call->getArgOperand(3));
+  uint64_t MemoryOrder(cast<Constant>(
+      Call->getArgOperand(4))->getUniqueInteger().getLimitedValue());
+
+  AtomicRMWInst::BinOp RMWOp;
+  switch (Op) {
+    default: RMWOp = AtomicRMWInst::BAD_BINOP; break;
+    case NaCl::AtomicAdd:  RMWOp = AtomicRMWInst::Add;  break;
+    case NaCl::AtomicSub:  RMWOp = AtomicRMWInst::Sub;  break;
+    case NaCl::AtomicOr:   RMWOp = AtomicRMWInst::Or;   break;
+    case NaCl::AtomicAnd:  RMWOp = AtomicRMWInst::And;  break;
+    case NaCl::AtomicXor:  RMWOp = AtomicRMWInst::Xor;  break;
+    case NaCl::AtomicXchg: RMWOp = AtomicRMWInst::Xchg; break;
+  }
+  AtomicOrdering Order;
+  switch (MemoryOrder) {
+    default:
+      // Conservatively use the strongest ordering.
+      Order = SequentiallyConsistent; break;
+    case NaCl::MemoryOrderRelaxed:        Order = Unordered; break;
+    case NaCl::MemoryOrderConsume:
+      // TODO Unspecified by LLVM's internal IR.
+      Order = SequentiallyConsistent; break;
+    case NaCl::MemoryOrderAcquire:        Order = Acquire; break;
+    case NaCl::MemoryOrderRelease:        Order = Release; break;
+    case NaCl::MemoryOrderAcquireRelease: Order = AcquireRelease; break;
+    case NaCl::MemoryOrderSequentiallyConsistent:
+      Order = SequentiallyConsistent; break;
+  }
+  const Twine Name("");
+  bool isVolatile = false;
+  SynchronizationScope SS = CrossThread;
+  bool hasUses;
+  Instruction *I;
+
+  for (size_t II = 0; II != NaCl::NumAtomicIntrinsics; ++II) {
+    if (IntrinsicID != NaCl::AtomicIntrinsics[II].ID)
+      continue;
+
+    unsigned BitSize = NaCl::AtomicIntrinsics[II].BitSize;
+    unsigned Align = 1 << (CountTrailingZeros_32(BitSize) - 3);
+    switch (Op) {
+      default: llvm_unreachable("invalid atomic operation");
+      case NaCl::AtomicLoad:
+        hasUses = true;
+        I = new LoadInst(Ptr, Name, isVolatile, Align, Order, SS, Call);
+        break;
+      case NaCl::AtomicStore:
+        hasUses = false;
+        I = new StoreInst(Val, Ptr, isVolatile, Align, Order, SS, Call);
+        break;
+      case NaCl::AtomicAdd:
+      case NaCl::AtomicSub:
+      case NaCl::AtomicOr:
+      case NaCl::AtomicAnd:
+      case NaCl::AtomicXor:
+      case NaCl::AtomicXchg:
+        hasUses = true;
+        I = new AtomicRMWInst(RMWOp, Ptr, Val, Order, SS, Call);
+        break;
+      case NaCl::AtomicCmpXchg:
+        hasUses = true;
+        I = new AtomicCmpXchgInst(Ptr, Old, Val, Order, SS, Call);
+        break;
+      case NaCl::AtomicFence:
+        hasUses = false;
+        I = new FenceInst(M->getContext(), Order, SS, Call);
+        break;
+    }
+    I->setDebugLoc(Call->getDebugLoc());
+    if (hasUses)

[Mark Seaborn, 2013/06/26 14:33:41]

I think you can do the RAUW unconditionally

[JF, 2013/06/26 15:52:29]

No:
  assert(New->getType() == getType() &&
         "replaceAllUses of value with new value of different type!");

+      Call->replaceAllUsesWith(I);
+    Call->eraseFromParent();
+  }
+}
+
 bool ResolvePNaClIntrinsics::runOnFunction(Function &F) {
-  bool Changed = resolveSimpleCall(F, Intrinsic::nacl_setjmp, "setjmp");
-  Changed |= resolveSimpleCall(F, Intrinsic::nacl_longjmp, "longjmp");
+  bool Changed = resolveSimpleCall(F, Intrinsic::nacl_setjmp);
+  Changed |= resolveSimpleCall(F, Intrinsic::nacl_longjmp);
+  for (size_t II = 0; II != NaCl::NumAtomicIntrinsics; ++II) {
+    Changed |= resolveSimpleCall(F, NaCl::AtomicIntrinsics[II].ID);
+  }
   return Changed;
 }