PNaCl: Impl the other atomicrmw operations: nand, max, min, umax, and umin.

lib/Transforms/NaCl/RewriteAtomics.cpp

Index: lib/Transforms/NaCl/RewriteAtomics.cpp
diff --git a/lib/Transforms/NaCl/RewriteAtomics.cpp b/lib/Transforms/NaCl/RewriteAtomics.cpp
index 8aafe3d13d3f1615da60f2e26e03e3b8e7bab6cf..b9d983e1abb2df2b10e254876cbb600c7a6ce40d 100644
--- a/lib/Transforms/NaCl/RewriteAtomics.cpp
+++ b/lib/Transforms/NaCl/RewriteAtomics.cpp
@@ -15,6 +15,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/Triple.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
@@ -28,6 +29,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/TargetRegistry.h"
 #include "llvm/Transforms/NaCl.h"
 #include <climits>
 #include <string>
@@ -169,9 +171,45 @@ INITIALIZE_PASS(RewriteAtomics, "nacl-rewrite-atomics",
                 false, false)
 
 bool RewriteAtomics::runOnModule(Module &M) {
+  // rewrite nand, (u)max, (u)min rmw atomics:
+  // First we need a target machine to appease its lordship:
+
+  // Get the target specific parser.
+  std::string Error;
+  Triple TheTriple = Triple("i686-none-nacl");

[JF, 2015/02/16 21:09:01]

Does it need to be i686? Can you just have unknown or pnacl instead?

[Richard Diamond, 2015/02/16 23:36:31]

The other option is `x86_64`. Arm expands into some kind of target specific
llvm.arm.* combo (I didn't really examine it closely), and mips doesn't need to
expand anything (supported natively, I assume). AtomicExpandPass queries the
TargetMachine to determine what to expand/what to expand to/etc and without one
it won't do anything.

Of the three mentioned, only X86 expands those atomic RMWs to target independant
IR. (If this seems smelly to you, I 100% agree).

+  const Target *TheTarget = TargetRegistry::lookupTarget("", TheTriple,
+                                                         Error);
+  if (!TheTarget) {
+    errs() << "Looking up 'i686-none-nacl':" << ": " << Error;
+    report_fatal_error("Did you forget to initialize the x86 target?");
+  }
+
+  // Create the target machine:
+  std::unique_ptr<TargetMachine> target(
+      TheTarget->createTargetMachine(TheTriple.getTriple(), "generic", "i686",
+                                     TargetOptions(), Reloc::Default,
+                                     CodeModel::Default, CodeGenOpt::Default));
+
+  std::unique_ptr<FunctionPass> AtomicRMWExpander
+    (createAtomicExpandPass(target.get()));
+
+
   AtomicVisitor AV(M, *this);
   AV.visit(M);
-  return AV.modifiedModule();
+
+  bool Changed = AV.modifiedModule();
+
+  // Expand any leftover RMW atomics:
+  // This is done after because otherwise -atomic-expand will expand stuff we're
+  // capable of expanding, leaving us with less efficient code.
+  for(auto &F : M.functions()) {
+    if (AtomicRMWExpander->runOnFunction(F)) {
+      // revisit the function, rewriting cmpxchg to the corresponding
+      // llvm.nacl.etc.etc.
+      AV.visit(F);
+    }
+  }

[JF, 2015/02/16 21:09:01]

Could you invoke the -atomic-expand pass after this one, and then re-run this
one, from the pass registry instead of manually?

It also looks like the Changed flag won't be quite right (well, technically it
is because any -atomic-expand rewrite will trigger a rewrite atomic after, but
still...).

[Richard Diamond, 2015/02/16 23:36:31]

No, because it would force this kluge onto users. This hack requires a specific
TargetMachine to work.

Apropos to Changed flag: whoops, fixed.

+  return Changed;
 }
 
 template <class Instruction>
@@ -354,7 +392,7 @@ void AtomicVisitor::visitStoreInst(StoreInst &I) {
 void AtomicVisitor::visitAtomicRMWInst(AtomicRMWInst &I) {
   NaCl::AtomicRMWOperation Op;
   switch (I.getOperation()) {
-  default: report_fatal_error("unsupported atomicrmw operation: " + ToStr(I));
+  default: return; // -atomic-expand will handle it.

[JF, 2015/02/16 21:09:01]

It would be good to assert that the second pass of this never hits this case.

[Richard Diamond, 2015/02/16 23:36:31]

The second pass never reaches this. The other RMW ops are expanded to cmpxchg
instructions.

   case AtomicRMWInst::Add: Op = NaCl::AtomicAdd; break;
   case AtomicRMWInst::Sub: Op = NaCl::AtomicSub; break;
   case AtomicRMWInst::And: Op = NaCl::AtomicAnd; break;