Subzero, Wasm: Link and run torture tests; bug fixes.

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.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 40 matching lines @@
 using namespace Ice;
 using namespace v8;
 using namespace v8::internal;
 using namespace v8::internal::wasm;
 using v8::internal::wasm::DecodeWasmModule;
 
 #undef LOG
 #define LOG(Expr) log([&](Ostream & out) { Expr; })
 
 namespace {
+// 64KB
+const uint32_t WASM_PAGE_SIZE = 64 << 10;
 
 std::string toStdString(WasmName Name) {
   return std::string(Name.name, Name.length);
 }
 
 Ice::Type toIceType(wasm::LocalType Type) {
   switch (Type) {
   case MachineRepresentation::kNone:
     llvm::report_fatal_error("kNone type not supported");
   case MachineRepresentation::kBit:
@@ skipping 234 matching lines @@
     return OperandNode(MergedNode);
   }
   Node Phi(wasm::LocalType, uint32_t Count, Node *Vals, Node Control) {
     LOG(out << "Phi(" << Count << ", " << Control);
     for (uint32_t i = 0; i < Count; ++i) {
       LOG(out << ", " << Vals[i]);
     }
     LOG(out << ") = ");
 
     const auto &InEdges = Control.toCfgNode()->getInEdges();
-    assert(Count == InEdges.size());
+    //assert(Count == InEdges.size());

[John, 2016/04/18 19:15:54]

why?

[Eric Holk, 2016/04/18 20:57:24]

This is leftover from one of my ill-fated debugging adventures. I've re-enabled
the assert.

The thinking was that Phi nodes are sometimes built incrementally by the WASM
decoder, so it may not always know all of the incoming edges to start with.

 
     assert(Count > 0);
 
     auto *Dest = makeVariable(Vals[0].toOperand()->getType(), Control);
 
     // Multiply by 10 in case more things get added later.
 
     // TODO(eholk): find a better way besides multiplying by some arbitrary
     // constant.
     auto *Phi = InstPhi::create(Func, Count * 10, Dest);
     for (uint32_t i = 0; i < Count; ++i) {
       auto *Op = Vals[i].toOperand();
       assert(Op);
       Phi->addArgument(Op, InEdges[i]);
     }
     setDefiningInst(Dest, Phi);
-    Control.toCfgNode()->appendInst(Phi);
+    constexpr bool AllowPhisAnywhere = true;

[John, 2016/04/18 19:15:54]

Why is this needed? It seems weird to me to have a phi instruction in the middle
of a basic block...

[Eric Holk, 2016/04/18 20:57:25]

Another relic of a misguided debugging adventure. I went ahead and removed this
parameter, since I haven't actually found it useful.

The thinking was that the decoder may not know all the incoming edges to a node
ahead of time, so later on in the process it might have to add additional phis
to a block. Because the CfgNode class separates Phis from regular instructions,
they would still end up at the beginning. This flag simply let you add more Phis
after you'd already started adding normal instructions.

+    Control.toCfgNode()->appendInst(Phi, AllowPhisAnywhere);
     LOG(out << Node(Dest) << "\n");
     return OperandNode(Dest);
   }
   Node EffectPhi(uint32_t Count, Node *Effects, Node Control) {
     // TODO(eholk): this function is almost certainly wrong.
     LOG(out << "EffectPhi(" << Count << ", " << Control << "):\n");
     for (uint32_t i = 0; i < Count; ++i) {
       LOG(out << "  " << Effects[i] << "\n");
     }
     return OperandNode(nullptr);
@@ skipping 116 matching lines @@
     case kExprI64And:
       Control()->appendInst(
           InstArithmetic::create(Func, InstArithmetic::And, Dest, Left, Right));
       break;
     case kExprI32Ne:
     case kExprI64Ne: {
       auto *TmpDest = makeVariable(IceType_i1);
       Control()->appendInst(
           InstIcmp::create(Func, InstIcmp::Ne, TmpDest, Left, Right));
       Control()->appendInst(
-          InstCast::create(Func, InstCast::Sext, Dest, TmpDest));
+          InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
       break;
     }
     case kExprI32Eq:
     case kExprI64Eq: {
       auto *TmpDest = makeVariable(IceType_i1);
       Control()->appendInst(
           InstIcmp::create(Func, InstIcmp::Eq, TmpDest, Left, Right));
       Control()->appendInst(
-          InstCast::create(Func, InstCast::Sext, Dest, TmpDest));
+          InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
       break;
     }
     case kExprI32LtS:
     case kExprI64LtS: {
       auto *TmpDest = makeVariable(IceType_i1);
       Control()->appendInst(
           InstIcmp::create(Func, InstIcmp::Slt, TmpDest, Left, Right));
       Control()->appendInst(
-          InstCast::create(Func, InstCast::Sext, Dest, TmpDest));
+          InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
       break;
     }
     case kExprI32LeS:
     case kExprI64LeS: {
       auto *TmpDest = makeVariable(IceType_i1);
       Control()->appendInst(
           InstIcmp::create(Func, InstIcmp::Sle, TmpDest, Left, Right));
       Control()->appendInst(
-          InstCast::create(Func, InstCast::Sext, Dest, TmpDest));
+          InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
       break;
     }
     case kExprI32GeU:
     case kExprI64GeU: {
       auto *TmpDest = makeVariable(IceType_i1);
       Control()->appendInst(
           InstIcmp::create(Func, InstIcmp::Uge, TmpDest, Left, Right));
       Control()->appendInst(
-          InstCast::create(Func, InstCast::Sext, Dest, TmpDest));
+          InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
       break;
     }
     case kExprI32LeU:
     case kExprI64LeU: {
       auto *TmpDest = makeVariable(IceType_i1);
       Control()->appendInst(
           InstIcmp::create(Func, InstIcmp::Ule, TmpDest, Left, Right));
       Control()->appendInst(
-          InstCast::create(Func, InstCast::Sext, Dest, TmpDest));
+          InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
       break;
     }
     case kExprI32LtU:
     case kExprI64LtU: {
       auto *TmpDest = makeVariable(IceType_i1);
       Control()->appendInst(
           InstIcmp::create(Func, InstIcmp::Ult, TmpDest, Left, Right));
       Control()->appendInst(
-          InstCast::create(Func, InstCast::Sext, Dest, TmpDest));
+          InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
       break;
     }
     case kExprI32GeS:
     case kExprI64GeS: {
       auto *TmpDest = makeVariable(IceType_i1);
       Control()->appendInst(
           InstIcmp::create(Func, InstIcmp::Sge, TmpDest, Left, Right));
       Control()->appendInst(
-          InstCast::create(Func, InstCast::Sext, Dest, TmpDest));
+          InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
     }
     case kExprI32GtS:
     case kExprI64GtS: {
       auto *TmpDest = makeVariable(IceType_i1);
       Control()->appendInst(
           InstIcmp::create(Func, InstIcmp::Sgt, TmpDest, Left, Right));
       Control()->appendInst(
-          InstCast::create(Func, InstCast::Sext, Dest, TmpDest));
+          InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
       break;
     }
     case kExprI32GtU:
     case kExprI64GtU: {
       auto *TmpDest = makeVariable(IceType_i1);
       Control()->appendInst(
           InstIcmp::create(Func, InstIcmp::Ugt, TmpDest, Left, Right));
       Control()->appendInst(
-          InstCast::create(Func, InstCast::Sext, Dest, TmpDest));
+          InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
       break;
     }
     case kExprF32Ne:
     case kExprF64Ne: {
       auto *TmpDest = makeVariable(IceType_i1);
       Control()->appendInst(
           InstFcmp::create(Func, InstFcmp::Une, TmpDest, Left, Right));
       Control()->appendInst(
-          InstCast::create(Func, InstCast::Sext, Dest, TmpDest));
+          InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
       break;
     }
     case kExprF32Le:
     case kExprF64Le: {
       auto *TmpDest = makeVariable(IceType_i1);
       Control()->appendInst(
           InstFcmp::create(Func, InstFcmp::Ule, TmpDest, Left, Right));
       Control()->appendInst(
-          InstCast::create(Func, InstCast::Sext, Dest, TmpDest));
+          InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
       break;
     }
     default:
       LOG(out << "Unknown binop: " << WasmOpcodes::OpcodeName(Opcode) << "\n");
       llvm::report_fatal_error("Uncovered or invalid binop.");
       return OperandNode(nullptr);
     }
     LOG(out << Dest << "\n");
     return OperandNode(Dest);
   }
   Node Unop(wasm::WasmOpcode Opcode, Node Input) {
     LOG(out << "Unop(" << WasmOpcodes::OpcodeName(Opcode) << ", " << Input
             << ") = ");
     Ice::Variable *Dest = nullptr;
     switch (Opcode) {
     case kExprI32Eqz: {
       Dest = makeVariable(IceType_i32);
       auto *Tmp = makeVariable(IceType_i1);
       Control()->appendInst(InstIcmp::create(Func, InstIcmp::Eq, Tmp, Input,
                                              Ctx->getConstantInt32(0)));
-      Control()->appendInst(InstCast::create(Func, InstCast::Sext, Dest, Tmp));
+      Control()->appendInst(InstCast::create(Func, InstCast::Zext, Dest, Tmp));
+      break;
+    }
+    case kExprI64Eqz: {
+      Dest = makeVariable(IceType_i32);

[John, 2016/04/18 19:15:54]

i1?

[Eric Holk, 2016/04/18 20:57:24]

Eqz corresponds to the C expression `x == 0`. It's pretty common for code to go
on and treat this as a 32-bit integer and expect it to be either 0 or 1. This is
why all the boolean operations in this file compare into a temporary i1 but then
zero-extend it to an i32. Branches then truncate before doing the branch.

This is on my list of things to improve, because a lot of times we end up with
zext immediately followed by a truncate.

[John, 2016/04/18 21:10:01]

That (zext followed by truncates) also plague pnacl bitcodes. :)

+      auto *Tmp = makeVariable(IceType_i1);
+      Control()->appendInst(InstIcmp::create(Func, InstIcmp::Eq, Tmp, Input,
+                                             Ctx->getConstantInt64(0)));
+      Control()->appendInst(InstCast::create(Func, InstCast::Zext, Dest, Tmp));
       break;
     }
     case kExprF32Neg: {
       Dest = makeVariable(IceType_f32);
       Control()->appendInst(InstArithmetic::create(
           Func, InstArithmetic::Fsub, Dest, Ctx->getConstantFloat(0), Input));
       break;
     }
     case kExprF64Neg: {
       Dest = makeVariable(IceType_f64);
@@ skipping 67 matching lines @@
     // save control here because true_node appears to alias control.
     auto *Ctrl = Control();
 
     *TrueNode = OperandNode(Func->makeNode());
     *FalseNode = OperandNode(Func->makeNode());
 
     LOG(out << *TrueNode << ", " << *FalseNode << ")"
             << "\n");
 
     auto *CondBool = makeVariable(IceType_i1);
-    Ctrl->appendInst(InstCast::create(Func, InstCast::Trunc, CondBool, Cond));
+    Ctrl->appendInst(InstIcmp::create(Func, InstIcmp::Ne, CondBool, Cond,
+                                      Ctx->getConstantInt32(0)));
 
     Ctrl->appendInst(InstBr::create(Func, CondBool, *TrueNode, *FalseNode));
     return OperandNode(nullptr);
   }
   InstSwitch *CurrentSwitch = nullptr;
   CfgNode *SwitchNode = nullptr;
   SizeT SwitchIndex = 0;
   Node Switch(uint32_t Count, Node Key) {
     LOG(out << "Switch(" << Count << ", " << Key << ")\n");
 
@@ skipping 204 matching lines @@
     (void)Index;
     llvm::report_fatal_error("LoadGlobal");
   }
   Node StoreGlobal(uint32_t Index, Node Val) {
     (void)Index;
     (void)Val;
     llvm::report_fatal_error("StoreGlobal");
   }
 
   Operand *sanitizeAddress(Operand *Base, uint32_t Offset) {
+    SizeT MemSize = Module->module->min_mem_pages * WASM_PAGE_SIZE;
+    SizeT MemMask = MemSize - 1;
+
+    bool ConstZeroBase = false;
+
     // first, add the index and the offset together.
-    if (0 != Offset) {
+    if (auto *ConstBase = llvm::dyn_cast<ConstantInteger32>(Base)) {
+      uint32_t RealOffset = Offset + ConstBase->getValue();
+      RealOffset &= MemMask;
+      Base = Ctx->getConstantInt32(RealOffset);
+      ConstZeroBase = (0 == RealOffset);
+    } else if (0 != Offset) {
       auto *Addr = makeVariable(IceType_i32);
       auto *OffsetConstant = Ctx->getConstantInt32(Offset);
       Control()->appendInst(InstArithmetic::create(Func, InstArithmetic::Add,
                                                    Addr, Base, OffsetConstant));
+
       Base = Addr;
     }
 
-    SizeT MemSize = Module->module->min_mem_pages * (16 << 10);
-    auto *WrappedAddr = makeVariable(IceType_i32);
-    Control()->appendInst(
-        InstArithmetic::create(Func, InstArithmetic::Add, WrappedAddr, Base,
-                               Ctx->getConstantInt32(MemSize)));
+    if (!llvm::dyn_cast<ConstantInteger32>(Base)) {
+      auto ClampedAddr = makeVariable(IceType_i32);

[John, 2016/04/18 19:15:54]

auto *

Also, maybe you should use

Ice::getPointerType() instead?

[Eric Holk, 2016/04/18 20:57:24]

Done.

I switch to Ice::getPointerType here and in the rest of the function too.

+      Control()->appendInst(
+          InstArithmetic::create(Func, InstArithmetic::And, ClampedAddr, Base,
+                                 Ctx->getConstantInt32(MemSize - 1)));
+      Base = ClampedAddr;
+    }
 
-    auto ClampedAddr = makeVariable(IceType_i32);
-    Control()->appendInst(
-        InstArithmetic::create(Func, InstArithmetic::And, ClampedAddr, Base,
-                               Ctx->getConstantInt32(MemSize - 1)));
+    Ice::Operand *RealAddr = nullptr;
+    auto MemBase = Ctx->getConstantSym(0, Ctx->getGlobalString("WASM_MEMORY"));
+    if (!ConstZeroBase) {
+      auto RealAddrV = Func->makeVariable(IceType_i32);
+      Control()->appendInst(InstArithmetic::create(
+          Func, InstArithmetic::Add, RealAddrV, Base, MemBase));
 
-    auto RealAddr = Func->makeVariable(IceType_i32);
-    auto MemBase = Ctx->getConstantSym(0, Ctx->getGlobalString("WASM_MEMORY"));
-    Control()->appendInst(InstArithmetic::create(
-        Func, InstArithmetic::Add, RealAddr, ClampedAddr, MemBase));
+      RealAddr = RealAddrV;
+    } else {
+      RealAddr = MemBase;
+    }
     return RealAddr;
   }
 
   Node LoadMem(wasm::LocalType Type, MachineType MemType, Node Index,
                uint32_t Offset) {
     LOG(out << "LoadMem(" << Index << "[" << Offset << "]) = ");
 
     auto *RealAddr = sanitizeAddress(Index, Offset);
 
     auto *LoadResult = makeVariable(toIceType(MemType));
@@ skipping 271 matching lines @@
       // Add the data
       WasmMemory->addInitializer(VariableDeclaration::DataInitializer::create(
           Globals.get(), reinterpret_cast<const char *>(Module->module_start) +
                              Seg.source_offset,
           Seg.source_size));
 
       WritePtr += Seg.source_size;
     }
 
     // Pad the rest with zeros
-    SizeT DataSize = Module->min_mem_pages * (64 << 10);
+    SizeT DataSize = Module->min_mem_pages * WASM_PAGE_SIZE;
     if (WritePtr < DataSize) {
       WasmMemory->addInitializer(VariableDeclaration::ZeroInitializer::create(
           Globals.get(), DataSize - WritePtr));
     }
 
-    WasmMemory->addInitializer(VariableDeclaration::ZeroInitializer::create(
-        Globals.get(), Module->min_mem_pages * (64 << 10)));
-
     Globals->push_back(WasmMemory);
 
     lowerGlobals(std::move(Globals));
   }
 
   // Translate each function.
   for (const auto Fn : Module->functions) {
     const auto FnName = getFunctionName(Module, Fn.func_index);
 
     LOG(out << "  " << Fn.func_index << ": " << FnName << "...");
 
     Body.sig = Fn.sig;
     Body.base = Buffer.get();
     Body.start = Buffer.get() + Fn.code_start_offset;
     Body.end = Buffer.get() + Fn.code_end_offset;
 
     auto Func = translateFunction(&Zone, Body);
     Func->setFunctionName(Ctx->getGlobalString(FnName));
 
     Ctx->optQueueBlockingPush(makeUnique<CfgOptWorkItem>(std::move(Func)));
     LOG(out << "done.\n");
   }
 
   return;
 }
 
 #endif // ALLOW_WASM