Initial Subzero WASM prototype.

src/WasmTranslator.cpp

+//===- subzero/src/WasmTranslator.cpp - WASM to Subzero Translation -------===//
+//
+//                        The Subzero Code Generator
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/StreamingMemoryObject.h"
+
+#include "WasmTranslator.h"
+
+#include "src/wasm/module-decoder.h"
+#include "src/wasm/wasm-opcodes.h"
+
+#include "src/zone.h"
+
+#include "IceGlobalInits.h"
+#include "IceCfgNode.h"
+
+using namespace std;
+using namespace Ice;
+using namespace v8;
+using namespace v8::internal;
+using namespace v8::internal::wasm;
+using v8::internal::wasm::DecodeWasmModule;
+
+#include "src/wasm/ast-decoder-impl.h"
+
+Ice::Type toIceType(v8::internal::MachineType) {
+  // TODO(eholk): actually convert this.
+  return IceType_i32;
+}
+
+Ice::Type toIceType(wasm::LocalType Type) {
+

[Jim Stichnoth, 2016/04/01 01:46:45]

remove leading blank line

[Eric Holk, 2016/04/01 19:15:02]

Done.

+  switch (Type) {
+  default:
+    llvm_unreachable("unexpected enum value");

[Jim Stichnoth, 2016/04/01 01:46:44]

Favor llvm::report_fatal_error over llvm_unreachable unless there's a good
reason.  IIRC, llvm_unreachable is more like assert() in that it may be compiled
out in a release build.

[Eric Holk, 2016/04/01 19:15:02]

Done.

+  case MachineRepresentation::kNone:
+    llvm_unreachable("kNone type not supported");
+  case MachineRepresentation::kBit:
+    return IceType_i1;
+  case MachineRepresentation::kWord8:
+    return IceType_i8;
+  case MachineRepresentation::kWord16:
+    return IceType_i16;
+  case MachineRepresentation::kWord32:
+    return IceType_i32;
+  case MachineRepresentation::kWord64:
+    return IceType_i64;
+  case MachineRepresentation::kFloat32:
+    return IceType_f32;
+  case MachineRepresentation::kFloat64:
+    return IceType_f64;
+  case MachineRepresentation::kSimd128:
+    llvm_unreachable("ambiguous SIMD type");
+  case MachineRepresentation::kTagged:
+    llvm_unreachable("kTagged type not supported");
+  }
+}
+
+/**

[Jim Stichnoth, 2016/04/01 01:46:44]

Subzero block commenting style is usually "//" prefixed lines.

[Eric Holk, 2016/04/01 19:15:02]

Done.

+   This class wraps either an Operand or a CfgNode.
+
+   Turbofan's sea of nodes representation only has nodes for values, control
+   flow, etc. In Subzero these concepts are all separate. This class let's V8's

[Jim Stichnoth, 2016/04/01 01:46:45]

lets

[Eric Holk, 2016/04/01 19:15:02]

Done.

+   Wasm decoder treat Subzero objects as though they are all the same.
+ */
+class OperandNode {
+  static constexpr uintptr_t NODE_FLAG = 1;
+  static constexpr uintptr_t UNDEF_PTR = (uintptr_t)-1;
+
+public:
+  uintptr_t Data;

[Jim Stichnoth, 2016/04/01 01:46:45]

Maybe use "uintptr_t Data = UNDEF_PTR;"?  Then you can just use
"OperandNode(...) = default;" for two of the ctors.

[Eric Holk, 2016/04/01 19:15:02]

Done.

The compiler still wouldn't accept default for the nullptr version though.

+
+  OperandNode() : Data(UNDEF_PTR) {}
+  OperandNode(Operand *Operand) : Data((uintptr_t)Operand) {}

[Jim Stichnoth, 2016/04/01 01:46:44]

I think this sort of casting would be better off as a reinterpret_cast or
whatever.  (Not so much the constexpr above, though.)

[Jim Stichnoth, 2016/04/01 01:46:45]

mark these ctors explicit

[Eric Holk, 2016/04/01 19:15:02]

Done.

This required a little bit more change on the V8 side than I really wanted, but
oh well.

[Eric Holk, 2016/04/01 19:15:02]

Done.

+  OperandNode(CfgNode *Node) : Data((uintptr_t)Node | NODE_FLAG) {}
+  OperandNode(nullptr_t) : Data(UNDEF_PTR) {}
+
+  operator Operand *() const {
+    if (UNDEF_PTR == Data) {
+      return nullptr;
+    }
+    assert(isOperand());
+    return reinterpret_cast<Operand *>(Data);
+  }
+
+  operator CfgNode *() const {
+    if (UNDEF_PTR == Data) {
+      return nullptr;
+    }
+    assert(isCfgNode());
+    return reinterpret_cast<CfgNode *>(Data & ~NODE_FLAG);
+  }
+
+  explicit operator bool() const { return (Data != UNDEF_PTR) && Data; }
+  bool operator==(const OperandNode &Rhs) const {
+    return (Data == Rhs.Data) ||
+           (UNDEF_PTR == Data && (Rhs.Data == 0 || Rhs.Data == NODE_FLAG)) ||
+           (UNDEF_PTR == Rhs.Data && (Data == 0 || Data == NODE_FLAG));
+  }
+  bool operator!=(const OperandNode &Rhs) const { return !(*this == Rhs); }
+
+  bool isOperand() const { return !(Data & NODE_FLAG); }
+  bool isCfgNode() const { return Data & NODE_FLAG; }

[Jim Stichnoth, 2016/04/01 01:46:45]

So UNDEF_PTR is considered to be a CfgNode and not an Operand?  That's oddly
asymmetric.

Without having read further into this file, I wonder if it's more appropriate
for both methods to return false on UNDEF_PTR.

[Eric Holk, 2016/04/01 19:15:02]

Good catch. Your suggestion was actually the intended behavior, and making that
change didn't break anything.

Done.

+};
+
+Ostream &operator<<(Ostream &Out, const OperandNode &Op) {
+  if (Op.isOperand()) {
+    auto Oper = static_cast<Operand *>(Op);

[Jim Stichnoth, 2016/04/01 01:46:44]

auto *

[Jim Stichnoth, 2016/04/01 01:46:45]

There are an awful lot of static_casts in this file.  What do you think of
having toOperand(), toNode(), etc. helpers to factor out all those static_casts?

[Eric Holk, 2016/04/01 19:15:02]

I think John suggested this too, and I've now seen the wisdom of this approach.
I'll make the change.

Done.

[Eric Holk, 2016/04/01 19:15:02]

Done.

+    Out << "(Operand*)" << Oper;
+  } else if (Op.isCfgNode()) {
+    auto Node = static_cast<CfgNode *>(Op);

[Jim Stichnoth, 2016/04/01 01:46:44]

auto *

[Eric Holk, 2016/04/01 19:15:02]

Done.

+    Out << "(CfgNode*)" << Node;
+  } else {
+    Out << "nullptr";
+  }
+  return Out;
+}
+
+constexpr bool isComparison(wasm::WasmOpcode Opcode) {
+  switch (Opcode) {
+  case kExprI32Ne:
+  case kExprI64Ne:
+  case kExprI32Eq:
+  case kExprI64Eq:
+  case kExprI32LtS:
+  case kExprI64LtS:
+  case kExprI32LtU:
+  case kExprI64LtU:
+  case kExprI32GeS:
+  case kExprI64GeS:
+  case kExprI32GtS:
+  case kExprI64GtS:
+  case kExprI32GtU:
+  case kExprI64GtU:
+    return true;
+  default:
+    return false;
+  }
+}
+
+#define LOG(Expr) log([&](Ostream & out) { Expr; })

[Jim Stichnoth, 2016/04/01 01:46:44]

I would move macro definitions like this closer to the top, since it's used in
pretty much the whole file.

[Eric Holk, 2016/04/01 19:15:01]

Done.

+
+class IceBuilder {
+  using Node = OperandNode;
+
+  IceBuilder() = delete;
+  IceBuilder(const IceBuilder &) = delete;
+  IceBuilder &operator=(const IceBuilder &) = delete;
+
+public:
+  IceBuilder(class Cfg *Cfg)

[Jim Stichnoth, 2016/04/01 01:46:44]

explicit

[Jim Stichnoth, 2016/04/01 01:46:44]

Don't use a type name for a variable name.  We used to do that a bit, and would
sometimes get into trouble when certain patterns of new code were added, or
maybe g++ would be more picky, or ...

In any case, Subzero usually does "Cfg *Func".

[Eric Holk, 2016/04/01 19:15:01]

Done.

[Eric Holk, 2016/04/01 19:15:01]

Done.

+      : Cfg(Cfg), Ctx(Cfg->getContext()), ControlPtr(nullptr) {}
+
+  /// Allocates a buffer of Nodes for use by V8.
+  Node *Buffer(size_t Count) {

[Jim Stichnoth, 2016/04/01 01:46:44]

All these method names should start with lowercase.  Sorry.

Or are you cloning a V8 or WebAsm API?

[Eric Holk, 2016/04/01 19:15:02]

Right, these are called by V8, so we have to follow their naming conventions.

+    LOG(out << "Buffer(" << Count << ")\n");
+    return Cfg->allocateArrayOf<Node>(Count);
+  }
+
+  Node Error() { llvm_unreachable("Error"); }
+  Node Start(unsigned params) {

[Jim Stichnoth, 2016/04/01 01:46:44]

capitalize Params

[Eric Holk, 2016/04/01 19:15:01]

Done.

+    LOG(out << "Start(" << params << ") = ");
+    auto *Entry = Cfg->makeNode();
+    Cfg->setEntryNode(Entry);
+    LOG(out << Node(Entry) << "\n");
+    return Entry;
+  }
+  Node Param(unsigned Index, wasm::LocalType Type) {
+    LOG(out << "Param(" << Index << ") = ");
+    auto *Arg = makeVariable(toIceType(Type));
+    assert(Index == NextArg);
+    Cfg->addArg(Arg);
+    NextArg++;

[Jim Stichnoth, 2016/04/01 01:46:44]

I think it's best to just get used to pre-incrementing everywhere, unless you
explicitly need post-increment.

[Eric Holk, 2016/04/01 19:15:01]

Done.

+    LOG(out << Node(Arg) << "\n");
+    return Arg;
+  }
+  CfgNode *Loop(CfgNode *Entry) {
+    auto *Loop = Cfg->makeNode();
+    LOG(out << "Loop(" << Entry << ") = " << Loop << "\n");
+    Entry->appendInst(InstBr::create(Cfg, Loop));
+    return Loop;
+  }
+  void Terminate(Node Effect, Node Control) {
+    // TODO(eholk): this is almost certainly wrong
+    LOG(out << "Terminate(" << Effect << ", " << Control << ")"
+            << "\n");
+  }
+  Node Merge(unsigned Count, Node *Controls) {
+    LOG(out << "Merge(" << Count);
+    for (unsigned i = 0; i < Count; ++i) {
+      LOG(out << ", " << Controls[i]);
+    }
+    LOG(out << ") = ");
+
+    auto *MergedNode = Cfg->makeNode();
+
+    for (unsigned i = 0; i < Count; ++i) {
+      CfgNode *Control = Controls[i];
+      Control->appendInst(InstBr::create(Cfg, MergedNode));
+    }
+    LOG(out << (OperandNode)MergedNode << "\n");
+    return MergedNode;
+  }
+  Node Phi(wasm::LocalType Type, unsigned Count, Node *Vals, Node Control) {
+    LOG(out << "Phi(" << Count << ", " << Control);
+    for (int i = 0; i < Count; ++i) {
+      LOG(out << ", " << Vals[i]);
+    }
+    LOG(out << ") = ");
+
+    const auto InEdges = static_cast<CfgNode *>(Control)->getInEdges();

[Jim Stichnoth, 2016/04/01 01:46:44]

const auto & (I think)

[Eric Holk, 2016/04/01 19:15:02]

Done.

+    assert(Count == InEdges.size());
+
+    assert(Count > 0);
+
+    auto *Dest =
+        makeVariable(static_cast<Operand *>(Vals[0])->getType(), Control);
+
+    // Multiply by 10 in case more things get added later.
+
+    // TODO(eholk): find a better way besides multiplying by some arbitrary
+    // constant.

[Jim Stichnoth, 2016/04/01 01:46:44]

Yeah -- here's where you eventually might just have to hold the phi data off to
the side until the CFG stabilizes.

[Eric Holk, 2016/04/01 19:15:02]

I was thinking about this some more, and it's possibly that multiplying by 2
might be enough to guarantee we always have enough space. The reason is that I
think Wasm's AST structure seems to make it so no block ever has more than two
incoming edges (at least I haven't seen any in my tests).

At any rate, I'd like to hold off on fixing this for a later CL.

+    auto *Phi = InstPhi::create(Cfg, Count * 10, Dest);
+    for (int i = 0; i < Count; ++i) {
+      auto *Op = static_cast<Operand *>(Vals[i]);
+      assert(Op);
+      Phi->addArgument(Op, InEdges[i]);
+    }
+    setDefiningInst(Dest, Phi);
+    static_cast<CfgNode *>(Control)->appendInst(Phi);
+    LOG(out << Node(Dest) << "\n");
+    return Dest;
+  }
+  Node EffectPhi(unsigned Count, Node *Effects, Node Control) {
+    // TODO(eholk): this function is almost certainly wrong.
+    LOG(out << "EffectPhi(" << Count << ", " << Control << "):\n");
+    for (unsigned i = 0; i < Count; ++i) {
+      LOG(out << "  " << Effects[i] << "\n");
+    }
+    return nullptr;
+  }
+  Node Int32Constant(int32_t Value) {
+    LOG(out << "Int32Constant(" << Value << ") = ");
+    auto *Const = Ctx->getConstantInt32(Value);
+    assert(Const);
+    assert(Control());
+    LOG(out << Node(Const) << "\n");
+    return Const;
+  }
+  Node Int64Constant(int64_t Value) {
+    LOG(out << "Int64Constant(" << Value << ") = ");
+    auto *Const = Ctx->getConstantInt64(Value);
+    assert(Const);
+    LOG(out << Node(Const) << "\n");
+    return Const;
+  }
+  Node Float32Constant(float Value) {
+    LOG(out << "Float32Constant(" << Value << ") = ");
+    auto *Const = Ctx->getConstantFloat(Value);
+    assert(Const);
+    LOG(out << Node(Const) << "\n");
+    return Const;
+  }
+  Node Float64Constant(double Value) {
+    LOG(out << "Float64Constant(" << Value << ") = ");
+    auto *Const = Ctx->getConstantDouble(Value);
+    assert(Const);
+    LOG(out << Node(Const) << "\n");
+    return Const;
+  }
+  Node Binop(wasm::WasmOpcode Opcode, Node Left, Node Right) {
+    LOG(out << "Binop(" << WasmOpcodes::OpcodeName(Opcode) << ", " << Left
+            << ", " << Right << ") = ");
+    auto *Dest = makeVariable(isComparison(Opcode)

[Jim Stichnoth, 2016/04/01 01:46:44]

Just checking - does wasm allow variables and functions to have explicit names? 
I see a lot of makeVariable() calls but no setName().

[Eric Holk, 2016/04/01 19:15:01]

I think imports and exports have explicit names (and we use those), but just
about everything else seems to be anonymous to me. I think there may be names
early on but they have been removed by the time we get here.

+                                  ? IceType_i1
+                                  : (static_cast<Operand *>(Left)->getType()));
+    switch (Opcode) {
+    case kExprI32Add:
+    case kExprI64Add:
+      Control()->appendInst(
+          InstArithmetic::create(Cfg, InstArithmetic::Add, Dest, Left, Right));
+      break;
+    case kExprI32Sub:
+    case kExprI64Sub:
+      Control()->appendInst(
+          InstArithmetic::create(Cfg, InstArithmetic::Sub, Dest, Left, Right));
+      break;
+    case kExprI32Mul:
+    case kExprI64Mul:
+      Control()->appendInst(
+          InstArithmetic::create(Cfg, InstArithmetic::Mul, Dest, Left, Right));
+      break;
+    case kExprI32DivU:
+    case kExprI64DivU:
+      Control()->appendInst(
+          InstArithmetic::create(Cfg, InstArithmetic::Udiv, Dest, Left, Right));
+      break;
+    case kExprI32RemU:
+    case kExprI64RemU:
+      Control()->appendInst(
+          InstArithmetic::create(Cfg, InstArithmetic::Urem, Dest, Left, Right));
+      break;
+    case kExprI32Ior:
+    case kExprI64Ior:
+      Control()->appendInst(
+          InstArithmetic::create(Cfg, InstArithmetic::Or, Dest, Left, Right));
+      break;
+    case kExprI32Xor:
+    case kExprI64Xor:
+      Control()->appendInst(
+          InstArithmetic::create(Cfg, InstArithmetic::Xor, Dest, Left, Right));
+      break;
+    case kExprI32Shl:
+    case kExprI64Shl:
+      Control()->appendInst(
+          InstArithmetic::create(Cfg, InstArithmetic::Shl, Dest, Left, Right));
+      break;
+    case kExprI32ShrU:
+    case kExprI64ShrU:
+    case kExprI32ShrS:
+    case kExprI64ShrS:
+      Control()->appendInst(
+          InstArithmetic::create(Cfg, InstArithmetic::Ashr, Dest, Left, Right));
+      break;
+    case kExprI32And:
+    case kExprI64And:
+      Control()->appendInst(
+          InstArithmetic::create(Cfg, InstArithmetic::And, Dest, Left, Right));
+      break;
+    case kExprI32Ne:
+    case kExprI64Ne:
+      Control()->appendInst(
+          InstIcmp::create(Cfg, InstIcmp::Ne, Dest, Left, Right));
+      break;
+    case kExprI32Eq:
+    case kExprI64Eq:
+      Control()->appendInst(
+          InstIcmp::create(Cfg, InstIcmp::Eq, Dest, Left, Right));
+      break;
+    case kExprI32LtS:
+    case kExprI64LtS:
+      Control()->appendInst(
+          InstIcmp::create(Cfg, InstIcmp::Slt, Dest, Left, Right));
+      break;
+    case kExprI32LtU:
+    case kExprI64LtU:
+      Control()->appendInst(
+          InstIcmp::create(Cfg, InstIcmp::Ult, Dest, Left, Right));
+      break;
+    case kExprI32GeS:
+    case kExprI64GeS:
+      Control()->appendInst(
+          InstIcmp::create(Cfg, InstIcmp::Sge, Dest, Left, Right));
+    case kExprI32GtS:
+    case kExprI64GtS:
+      Control()->appendInst(
+          InstIcmp::create(Cfg, InstIcmp::Sgt, Dest, Left, Right));
+      break;
+    case kExprI32GtU:
+    case kExprI64GtU:
+      Control()->appendInst(
+          InstIcmp::create(Cfg, InstIcmp::Ugt, Dest, Left, Right));
+      break;
+    default:
+      LOG(out << "Unknown binop: " << WasmOpcodes::OpcodeName(Opcode) << "\n");
+      llvm_unreachable("Uncovered or invalid binop.");
+      return nullptr;
+    }
+    LOG(out << Dest << "\n");
+    return Dest;
+  }
+  Node Unop(wasm::WasmOpcode Opcode, Node Input) {
+    LOG(out << "Unop(" << WasmOpcodes::OpcodeName(Opcode) << ", " << Input
+            << ") = ");
+    Ice::Variable *Dest = nullptr;
+    switch (Opcode) {
+    case kExprF32Neg: {
+      Dest = makeVariable(IceType_f32);
+      auto *Zero = makeVariable(IceType_f32);

[Jim Stichnoth, 2016/04/01 01:46:44]

auto *_0 = Ctx->getConstantZero(IceType_f32);
Control()->appendInst(InstArithmetic::create(Cfg, InstArithmetic::Fsub, Dest,
_0, Input));

I.e., you don't need to create a special variable that has 0 assigned to it.

[Eric Holk, 2016/04/01 19:15:02]

Right, I don't know why I thought you had to do it this way.

Done.

+      Control()->appendInst(
+          InstAssign::create(Cfg, Zero, Ctx->getConstantFloat(0)));
+      Control()->appendInst(
+          InstArithmetic::create(Cfg, InstArithmetic::Fsub, Dest, Zero, Input));
+      break;
+    }
+    case kExprF64Neg: {
+      Dest = makeVariable(IceType_f64);
+      auto *Zero = makeVariable(IceType_f64);
+      Control()->appendInst(
+          InstAssign::create(Cfg, Zero, Ctx->getConstantDouble(0)));
+      Control()->appendInst(
+          InstArithmetic::create(Cfg, InstArithmetic::Fsub, Dest, Zero, Input));
+      break;
+    }
+    case kExprI64UConvertI32:
+      Dest = makeVariable(IceType_i64);
+      Control()->appendInst(InstCast::create(Cfg, InstCast::Zext, Dest, Input));
+      break;
+    default:
+      LOG(out << "Unknown unop: " << WasmOpcodes::OpcodeName(Opcode) << "\n");
+      llvm_unreachable("Uncovered or invalid unop.");
+      return nullptr;
+    }
+    LOG(out << Dest << "\n");
+    return Dest;
+  }
+  unsigned InputCount(CfgNode *Node) { return Node->getInEdges().size(); }
+  bool IsPhiWithMerge(Node Phi, Node Merge) {
+    LOG(out << "IsPhiWithMerge(" << Phi << ", " << Merge << ")"

[Jim Stichnoth, 2016/04/01 01:46:44]

Did you run "make format"?  I'm surprised this statement spans two lines.

[Eric Holk, 2016/04/01 19:15:01]

I may have changed this since the last time I ran make format. I'll run again
before I update.

+            << "\n");
+    if (Phi && Phi.isOperand()) {
+      LOG(out << "  ...is operand"
+              << "\n");
+      if (auto *Inst = getDefiningInst(Phi)) {
+        LOG(out << "  ...has defining instruction"
+                << "\n");
+        LOG(out << getDefNode(Phi) << "\n");
+        LOG(out << "  ..." << (getDefNode(Phi) == Merge) << "\n");
+        return getDefNode(Phi) == Merge;
+      }
+    }
+    return false;
+  }
+  void AppendToMerge(CfgNode *Merge, CfgNode *From) const {
+    From->appendInst(InstBr::create(Cfg, Merge));
+  }
+  void AppendToPhi(Node Merge, Node Phi, Node From) {
+    LOG(out << "AppendToPhi(" << Merge << ", " << Phi << ", " << From << ")"
+            << "\n");
+    auto *Inst = getDefiningInst(Phi);
+    Inst->addArgument(From, getDefNode(From));
+  }
+
+  //-----------------------------------------------------------------------
+  // Operations that read and/or write {control} and {effect}.
+  //-----------------------------------------------------------------------
+  nullptr_t Branch(Node Cond, Node *TrueNode, Node *FalseNode) {
+    // true_node and false_node appear to be out parameters.
+    LOG(out << "Branch(" << Cond << ", ");
+
+    // save control here because true_node appears to alias control.
+    auto *Ctrl = Control();
+
+    *TrueNode = Cfg->makeNode();
+    *FalseNode = Cfg->makeNode();
+
+    LOG(out << *TrueNode << ", " << *FalseNode << ")"
+            << "\n");
+
+    Ctrl->appendInst(InstBr::create(Cfg, Cond, *TrueNode, *FalseNode));
+    return nullptr;
+  }
+  Node Switch(unsigned Count, Node Key) { llvm_unreachable("Switch"); }
+  Node IfValue(int32_t Value, Node Sw) { llvm_unreachable("IfValue"); }
+  Node IfDefault(Node Sw) { llvm_unreachable("IfDefault"); }
+  Node Return(unsigned Count, Node *Vals) {
+    assert(1 >= Count);
+    LOG(out << "Return(");
+    if (Count > 0)
+      LOG(out << Vals[0]);
+    LOG(out << ")"
+            << "\n");
+    auto *Instr =
+        1 == Count ? InstRet::create(Cfg, Vals[0]) : InstRet::create(Cfg);
+    Control()->appendInst(Instr);
+    Control()->setHasReturn();
+    LOG(out << Node(nullptr) << "\n");
+    return nullptr;
+  }
+  Node ReturnVoid() {
+    LOG(out << "ReturnVoid() = ");
+    auto *Instr = InstRet::create(Cfg);
+    Control()->appendInst(Instr);
+    Control()->setHasReturn();
+    LOG(out << Node(nullptr) << "\n");
+    return nullptr;
+  }
+  Node Unreachable() {
+    LOG(out << "Unreachable() = ");
+    auto *Instr = InstUnreachable::create(Cfg);
+    Control()->appendInst(Instr);
+    LOG(out << Node(nullptr) << "\n");
+    return nullptr;
+  }
+
+  Node CallDirect(uint32_t Index, Node *Args) {
+    LOG(out << "CallDirect(" << Index << ")"
+            << "\n");
+    assert(Module->IsValidFunction(Index));
+    const auto *Module = this->Module->module;
+    assert(Module);
+    const auto &Target = Module->functions[Index];
+    const auto *Sig = Target.sig;
+    assert(Sig);
+    const auto NumArgs = Sig->parameter_count();
+    LOG(out << "  number of args: " << NumArgs << "\n");
+
+    const auto TargetName = Ctx->getGlobalString(
+        Module->GetName(Target.name_offset));
+    LOG(out << "  target name: " << TargetName << "\n");
+
+    assert(Sig->return_count() <= 1);
+
+    auto *TargetOperand = Ctx->getConstantSym(0, TargetName);
+
+    auto *Dest = Sig->return_count() > 0
+                     ? makeVariable(toIceType(Sig->GetReturn()))
+                     : nullptr;
+    auto *Call = InstCall::create(Cfg, NumArgs, Dest, TargetOperand,
+                                  false /* HasTailCall */);
+    for (int i = 0; i < NumArgs; ++i) {
+      // The builder reserves the first argument for the code object.
+      LOG(out << "  args[" << i << "] = " << Args[i + 1] << "\n");
+      Call->addArg(Args[i + 1]);
+    }
+
+    Control()->appendInst(Call);
+    LOG(out << "Call Result = " << Node(Dest) << "\n");
+    return Dest;
+  }
+  Node CallImport(uint32_t Index, Node *Args) {
+    LOG(out << "CallImport(" << Index << ")"
+            << "\n");
+    // assert(Module->IsValidFunction(index));

[Jim Stichnoth, 2016/04/01 01:46:44]

remove this?

[Eric Holk, 2016/04/01 19:15:02]

Done.

+    const auto *Module = this->Module->module;
+    assert(Module);
+    const auto *Sig = this->Module->GetImportSignature(Index);
+    assert(Sig);
+    const auto NumArgs = Sig->parameter_count();
+    LOG(out << "  number of args: " << NumArgs << "\n");
+
+    const auto &Target = Module->import_table[Index];
+    const auto TargetName = Ctx->getGlobalString(
+        Module->GetName(Target.function_name_offset));
+    LOG(out << "  target name: " << TargetName << "\n");
+
+    assert(Sig->return_count() <= 1);
+
+    auto *TargetOperand = Ctx->getConstantSym(0, TargetName);
+
+    auto *Dest = Sig->return_count() > 0
+                     ? makeVariable(toIceType(Sig->GetReturn()))
+                     : nullptr;
+    auto *Call = InstCall::create(Cfg, NumArgs, Dest, TargetOperand,

[Jim Stichnoth, 2016/04/01 01:46:45]

Subzero tends to document constant arguments like:

constexpr bool NoTailCall = false;
InstCall::create(..., NoTailCall);

[Eric Holk, 2016/04/01 19:15:02]

Done.

+                                  false /* HasTailCall */);
+    for (int i = 0; i < NumArgs; ++i) {
+      // The builder reserves the first argument for the code object.
+      LOG(out << "  args[" << i << "] = " << Args[i + 1] << "\n");
+      Call->addArg(Args[i + 1]);
+    }
+
+    Control()->appendInst(Call);
+    LOG(out << "Call Result = " << Node(Dest) << "\n");
+    return Dest;
+  }
+  Node CallIndirect(uint32_t Index, Node *Args) {
+    llvm_unreachable("CallIndirect");
+  }
+  Node Invert(Node Node) { llvm_unreachable("Invert"); }
+  Node FunctionTable() { llvm_unreachable("FunctionTable"); }
+
+  //-----------------------------------------------------------------------
+  // Operations that concern the linear memory.
+  //-----------------------------------------------------------------------
+  Node MemSize(uint32_t Offset) { llvm_unreachable("MemSize"); }
+  Node LoadGlobal(uint32_t Index) { llvm_unreachable("LoadGlobal"); }
+  Node StoreGlobal(uint32_t Index, Node Val) {
+    llvm_unreachable("StoreGlobal");
+  }
+  Node LoadMem(wasm::LocalType Type, MachineType MemType, Node Index,
+               uint32_t Offset) {
+    LOG(out << "LoadMem(" << Index << "[" << Offset << "]) = ");
+
+    // TODO(eholk): surely there is a better way to do this.

[Jim Stichnoth, 2016/04/01 01:46:44]

Probably not - LLVM/PNaCl representation is pretty low-level here...

[Eric Holk, 2016/04/01 19:15:02]

Okay, I'll remove the comment.

+
+    // first, add the index and the offset together.
+    auto *OffsetConstant = Ctx->getConstantInt32(Offset);
+    auto *Addr = makeVariable(IceType_i32);
+    Control()->appendInst(InstArithmetic::create(Cfg, InstArithmetic::Add, Addr,
+                                                 Index, OffsetConstant));
+
+    // then load the memory
+    auto *LoadResult = makeVariable(toIceType(MemType));
+    Control()->appendInst(InstLoad::create(Cfg, LoadResult, Addr));
+
+    // and cast, if needed
+    Ice::Variable *Result = nullptr;
+    if (toIceType(Type) != toIceType(MemType)) {
+      Result = makeVariable(toIceType(Type));
+      // TODO(eholk): handle signs correctly.
+      Control()->appendInst(
+          InstCast::create(Cfg, InstCast::Sext, Result, LoadResult));
+    } else {
+      Result = LoadResult;
+    }
+
+    LOG(out << Result << "\n");
+    return Result;
+  }
+  void StoreMem(MachineType Type, Node Index, uint32_t Offset, Node Val) {
+    LOG(out << "StoreMem(" << Index << "[" << Offset << "] = " << Val << ")"
+            << "\n");
+
+    // TODO(eholk): surely there is a better way to do this.
+
+    // first, add the index and the offset together.
+    auto *OffsetConstant = Ctx->getConstantInt32(Offset);
+    auto *Addr = makeVariable(IceType_i32);
+    Control()->appendInst(InstArithmetic::create(Cfg, InstArithmetic::Add, Addr,
+                                                 Index, OffsetConstant));
+
+    // cast the value to the right type, if needed
+    Operand *StoreVal = nullptr;
+    if (toIceType(Type) != static_cast<Operand *>(Val)->getType()) {
+      auto *LocalStoreVal = makeVariable(toIceType(Type));
+      Control()->appendInst(
+          InstCast::create(Cfg, InstCast::Trunc, LocalStoreVal, Val));
+      StoreVal = LocalStoreVal;
+    } else {
+      StoreVal = Val;
+    }
+
+    // then store the memory
+    Control()->appendInst(InstStore::create(Cfg, StoreVal, Addr));
+  }
+
+  static void PrintDebugName(Node node) { llvm_unreachable("PrintDebugName"); }
+
+  CfgNode *Control() {
+    return ControlPtr ? static_cast<CfgNode *>(*ControlPtr)
+                      : Cfg->getEntryNode();
+  }
+  Node Effect() { return *EffectPtr; }
+
+  void set_module(wasm::ModuleEnv *Module) { this->Module = Module; }

[Jim Stichnoth, 2016/04/01 01:46:45]

The LLVM/Subzero naming convention would be more like setModule().

[Eric Holk, 2016/04/01 19:15:01]

This is another function called by V8 so we have to use their name here.

+
+  void set_control_ptr(Node *Control) { this->ControlPtr = Control; }
+
+  void set_effect_ptr(Node *Effect) { this->EffectPtr = Effect; }
+
+private:
+  wasm::ModuleEnv *Module;
+  Node *ControlPtr;
+  Node *EffectPtr;
+
+  class Cfg *Cfg;
+  GlobalContext *Ctx;
+
+  SizeT NextArg = 0;
+
+  CfgUnorderedMap<Operand *, InstPhi *> PhiMap;
+  CfgUnorderedMap<Operand *, CfgNode *> DefNodeMap;
+
+  InstPhi *getDefiningInst(Operand *Op) const {
+    const auto &Iter = PhiMap.find(Op);
+    if (Iter == PhiMap.end()) {
+      return nullptr;
+    }
+    return Iter->second;
+  }
+
+  void setDefiningInst(Operand *Op, InstPhi *Phi) {
+    LOG(out << "\n== setDefiningInst(" << Op << ", " << Phi << ") ==\n");
+    PhiMap.emplace(Op, Phi);
+  }
+
+  Ice::Variable *makeVariable(Ice::Type Type) {
+    return makeVariable(Type, Control());
+  }
+
+  Ice::Variable *makeVariable(Ice::Type Type, CfgNode *DefNode) {
+    auto *Var = Cfg->makeVariable(Type);
+    DefNodeMap.emplace(Var, DefNode);
+    return Var;
+  }
+
+  CfgNode *getDefNode(Operand *Op) const {
+    const auto &Iter = DefNodeMap.find(Op);
+    if (Iter == DefNodeMap.end()) {
+      return nullptr;
+    }
+    return Iter->second;
+  }
+
+  template <typename F = std::function<void(Ostream &)>> void log(F Fn) {
+    if (BuildDefs::dump() && (Ctx->getFlags().getVerbose() & IceV_Wasm)) {
+      OstreamLocker L(Ctx);

[Jim Stichnoth, 2016/04/01 01:46:45]

This and the LOG() macro are clever.

The following is optional for this CL.  For verbose output at this level, I
think you really want to hold the Ostream lock for the entire duration of
parsing one function (and of course only in a DUMP-enabled build).  Otherwise,
when we get to the point where we let the translation threads also parse in
parallel, you end up with hopelessly interleaved parse log output.

This approach is already done elsewhere, including (off the top of my head)
register allocation and address mode inference.

[Eric Holk, 2016/04/01 19:15:01]

Hopefully that's a good clever :)
That seems like a good idea and a simple change, so I went ahead and did it.

+      Fn(Ctx->getStrDump());
+      Ctx->getStrDump().flush();
+    }
+  }
+};
+
+std::string fnNameFromId(uint32_t Id) {
+  return std::string("fn") + to_string(Id);
+}
+
+std::unique_ptr<Cfg> WasmTranslator::translateFunction(Zone *Zone,
+                                                       FunctionEnv *Env,
+                                                       const byte *Base,
+                                                       const byte *Start,
+                                                       const byte *End) {
+  auto Cfg = Cfg::create(Ctx, getNextSequenceNumber());
+  Ice::CfgLocalAllocatorScope _(Cfg.get());
+
+  // TODO: parse the function signature...
+
+  IceBuilder Builder(Cfg.get());
+  LR_WasmDecoder<OperandNode, IceBuilder> Decoder(Zone, &Builder);
+
+  LOG(out << Ctx->getFlags().getDefaultGlobalPrefix() << "\n");
+  Decoder.Decode(Env, Base, Start, End);
+
+  // We don't always know where the incoming branches are in phi nodes, so this
+  // function finds them.
+  Cfg->fixPhiNodes();
+
+  return Cfg;
+}
+
+WasmTranslator::WasmTranslator(GlobalContext *Ctx) : Translator(Ctx) {
+  // TODO(eholk): compute the correct buffer size.
+  BufferSize = 24 << 10; // 24k, bigger than the test cases I have so far.
+  Buffer = new uint8_t[BufferSize];
+}
+
+WasmTranslator::~WasmTranslator() {
+  if (Buffer) {
+    delete[] Buffer;

[Jim Stichnoth, 2016/04/01 01:46:44]

Can Buffer be a unique_ptr?  Then you don't need to bother deleting it here.

[Eric Holk, 2016/04/01 19:15:02]

I like that much better.

Done.

+  }
+}
+
+void WasmTranslator::translate(
+    const std::string &IRFilename,
+    std::unique_ptr<llvm::DataStreamer> InputStream) {
+  LOG(out << "Initializing v8/wasm stuff..."
+          << "\n");
+  Zone Zone;
+  ZoneScope _(&Zone);
+
+  SizeT BytesRead = InputStream->GetBytes(Buffer, BufferSize);
+  LOG(out << "Read " << BytesRead << " bytes"
+          << "\n");
+
+  LOG(out << "Decoding module " << IRFilename << "\n");
+
+  auto Result =
+      DecodeWasmModule(nullptr /* DecodeWasmModule ignores the Isolate
+                                          * parameter, so we just pass a null
+                                          * one rather than go through the
+                                          * hullabaloo of making one. */, &Zone,
+                       Buffer, Buffer + BytesRead, false, kWasmOrigin);
+
+  auto Module = Result.val;
+
+  LOG(out << "Module info:"
+          << "\n");
+  LOG(out << "  number of globals:       " << Module->globals.size() << "\n");
+  LOG(out << "  number of signatures:    " << Module->signatures.size()
+          << "\n");
+  LOG(out << "  number of functions:     " << Module->functions.size() << "\n");
+  LOG(out << "  number of data_segments: " << Module->data_segments.size()
+          << "\n");
+  LOG(out << "  function table size:     " << Module->function_table.size()
+          << "\n");
+
+  ModuleEnv ModuleEnv;
+  ModuleEnv.module = Module;
+
+  LOG(out << "\n"
+          << "Function information:"
+          << "\n");
+  for (const auto F : Module->functions) {
+    LOG(out << "  " << F.name_offset << ": " << Module->GetName(F.name_offset));
+    if (F.exported)
+      LOG(out << " export");
+    if (F.external)
+      LOG(out << " extern");
+    LOG(out << "\n");
+  }
+
+  FunctionEnv Fenv;
+  Fenv.module = &ModuleEnv;
+
+  LOG(out << "Translating " << IRFilename << "\n");
+
+  // Translate each function.
+  uint32_t Id = 0;
+  for (const auto Fn : Module->functions) {
+    std::string NewName = fnNameFromId(Id++);
+    LOG(out << "  " << Fn.name_offset << ": " << Module->GetName(Fn.name_offset)
+            << " -> " << NewName << "...");
+
+    Fenv.sig = Fn.sig;
+    Fenv.local_i32_count = Fn.local_i32_count;
+    Fenv.local_i64_count = Fn.local_i64_count;
+    Fenv.local_f32_count = Fn.local_f32_count;
+    Fenv.local_f64_count = Fn.local_f64_count;
+    Fenv.SumLocals();
+
+    auto Cfg =
+        translateFunction(&Zone, &Fenv, Buffer, Buffer + Fn.code_start_offset,
+                          Buffer + Fn.code_end_offset);
+    Cfg->setFunctionName(Ctx->getGlobalString(NewName));
+
+    Ctx->optQueueBlockingPush(std::move(Cfg));
+    LOG(out << "done."

[Jim Stichnoth, 2016/04/01 01:46:45]

Do one of these instead:

  out << "done.\n"
  out << "done." "\n"

(the latter being compiler string concatenation)

[Eric Holk, 2016/04/01 19:15:02]

Done.

+            << "\n");
+  }
+
+  return;
+}