Subzero, WASM: stop writing uninitialized data to .o file. Add timers.

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 258 matching lines @@
     LOG(out << "Buffer(" << Count << ")\n");
     return Func->allocateArrayOf<Node>(Count);
   }
 
   Node Error() { llvm::report_fatal_error("Error"); }
   Node Start(uint32_t Params) {
     LOG(out << "Start(" << Params << ") = ");
     auto *Entry = Func->getEntryNode();
     assert(Entry);
     LOG(out << Node(Entry) << "\n");
+
+    // Load the WasmMemory address to make it available everywhere else in the
+    // function.
+    auto *WasmMemoryPtr =
+        Ctx->getConstantExternSym(Ctx->getGlobalString("WASM_MEMORY"));
+    assert(!WasmMemory);

[Jim Stichnoth, 2016/05/02 16:04:07]

assert(WasmMemory == nullptr);

[Eric Holk, 2016/05/02 17:42:37]

Done.

+    auto *WasmMemoryV = makeVariable(getPointerType());
+    Entry->appendInst(InstLoad::create(Func, WasmMemoryV, WasmMemoryPtr));
+    WasmMemory = WasmMemoryV;
+
     return OperandNode(Entry);
   }
   Node Param(uint32_t Index, wasm::LocalType Type) {
     LOG(out << "Param(" << Index << ") = ");
     auto *Arg = makeVariable(toIceType(Type));
     assert(Index == NextArg);
     Func->addArg(Arg);
     ++NextArg;
     LOG(out << Node(Arg) << "\n");
     return OperandNode(Arg);
@@ skipping 866 matching lines @@
   Node LoadGlobal(uint32_t Index) {
     (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) {

[Eric Holk, 2016/04/29 22:37:42]

This function is just moved down lower to make it private.

-    SizeT MemSize = Module->module->min_mem_pages * WASM_PAGE_SIZE;
-
-    bool ConstZeroBase = false;
-
-    // first, add the index and the offset together.
-    if (auto *ConstBase = llvm::dyn_cast<ConstantInteger32>(Base)) {
-      uint32_t RealOffset = Offset + ConstBase->getValue();
-      if (RealOffset >= MemSize) {
-        // We've proven this will always be an out of bounds access, so insert
-        // an unconditional trap.
-        Control()->appendInst(InstUnreachable::create(Func));
-        // It doesn't matter what we return here, so return something that will
-        // allow the rest of code generation to happen.
-        //
-        // We might be tempted to just abort translation here, but out of bounds
-        // memory access is a runtime trap, not a compile error.
-        return Ctx->getConstantZero(getPointerType());
-      }
-      Base = Ctx->getConstantInt32(RealOffset);
-      ConstZeroBase = (0 == RealOffset);
-    } else if (0 != Offset) {
-      auto *Addr = makeVariable(Ice::getPointerType());
-      auto *OffsetConstant = Ctx->getConstantInt32(Offset);
-      Control()->appendInst(InstArithmetic::create(Func, InstArithmetic::Add,
-                                                   Addr, Base, OffsetConstant));
-
-      Base = Addr;
-    }
-
-    // Do the bounds check.
-    //
-    // TODO (eholk): Add a command line argument to control whether bounds
-    // checks are inserted, and maybe add a way to duplicate bounds checks to
-    // get a better sense of the overhead.
-    if (!llvm::dyn_cast<ConstantInteger32>(Base)) {
-      // TODO (eholk): creating a new basic block on every memory access is
-      // terrible (see https://goo.gl/Zj7DTr). Try adding a new instruction that
-      // encapsulates this "abort if false" pattern.
-      auto *CheckPassed = Func->makeNode();
-      auto *CheckFailed = getBoundsFailTarget();
-
-      auto *Check = makeVariable(IceType_i1);
-      Control()->appendInst(InstIcmp::create(Func, InstIcmp::Ult, Check, Base,
-                                             Ctx->getConstantInt32(MemSize)));
-      Control()->appendInst(
-          InstBr::create(Func, Check, CheckPassed, CheckFailed));
-
-      *ControlPtr = OperandNode(CheckPassed);
-    }
-
-    Ice::Operand *RealAddr = nullptr;
-    auto MemBase = Ctx->getConstantSym(0, Ctx->getGlobalString("WASM_MEMORY"));
-    if (!ConstZeroBase) {
-      auto RealAddrV = Func->makeVariable(Ice::getPointerType());
-      Control()->appendInst(InstArithmetic::create(Func, InstArithmetic::Add,
-                                                   RealAddrV, Base, MemBase));
-
-      RealAddr = RealAddrV;
-    } else {
-      RealAddr = MemBase;
-    }
-    return RealAddr;
-  }
-
   Node LoadMem(wasm::LocalType Type, MachineType MemType, Node Index,
                uint32_t Offset) {
     LOG(out << "LoadMem." << toIceType(MemType) << "(" << Index << "[" << Offset
             << "]) = ");
 
     auto *RealAddr = sanitizeAddress(Index, Offset);
 
     auto *LoadResult = makeVariable(toIceType(MemType));
     Control()->appendInst(InstLoad::create(Func, LoadResult, RealAddr));
 
@@ skipping 71 matching lines @@
   GlobalContext *Ctx;
 
   CfgNode *BoundsFailTarget = nullptr;
   CfgNode *IndirectFailTarget = nullptr;
 
   SizeT NextArg = 0;
 
   CfgUnorderedMap<Operand *, InstPhi *> PhiMap;
   CfgUnorderedMap<Operand *, CfgNode *> DefNodeMap;
 
+  Operand *WasmMemory = nullptr;
+
   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");
@@ skipping 40 matching lines @@
       IndirectFailTarget->appendInst(InstCall::create(
           Func, 0, nullptr,
           Ctx->getConstantExternSym(Ctx->getGlobalString("__Sz_indirect_fail")),
           false));
       IndirectFailTarget->appendInst(InstUnreachable::create(Func));
     }
 
     return IndirectFailTarget;
   }
 
+  Operand *getWasmMemory() {
+    assert(WasmMemory);

[Jim Stichnoth, 2016/05/02 16:04:07]

assert(WasmMemory != nullptr);

I think we're pretty consistent in the code base these days about explicitly
comparing pointers against nullptr.

[Eric Holk, 2016/05/02 17:42:37]

Done.

+    return WasmMemory;
+  }
+
+  Operand *sanitizeAddress(Operand *Base, uint32_t Offset) {
+    SizeT MemSize = Module->module->min_mem_pages * WASM_PAGE_SIZE;
+
+    bool ConstZeroBase = false;
+
+    // first, add the index and the offset together.
+    if (auto *ConstBase = llvm::dyn_cast<ConstantInteger32>(Base)) {
+      uint32_t RealOffset = Offset + ConstBase->getValue();
+      if (RealOffset >= MemSize) {
+        // We've proven this will always be an out of bounds access, so insert
+        // an unconditional trap.
+        Control()->appendInst(InstUnreachable::create(Func));
+        // It doesn't matter what we return here, so return something that will
+        // allow the rest of code generation to happen.
+        //
+        // We might be tempted to just abort translation here, but out of bounds
+        // memory access is a runtime trap, not a compile error.
+        return Ctx->getConstantZero(getPointerType());
+      }
+      Base = Ctx->getConstantInt32(RealOffset);
+      ConstZeroBase = (0 == RealOffset);
+    } else if (0 != Offset) {
+      auto *Addr = makeVariable(Ice::getPointerType());
+      auto *OffsetConstant = Ctx->getConstantInt32(Offset);
+      Control()->appendInst(InstArithmetic::create(Func, InstArithmetic::Add,
+                                                   Addr, Base, OffsetConstant));
+
+      Base = Addr;
+    }
+
+    // Do the bounds check.
+    //
+    // TODO (eholk): Add a command line argument to control whether bounds
+    // checks are inserted, and maybe add a way to duplicate bounds checks to
+    // get a better sense of the overhead.
+    if (!llvm::dyn_cast<ConstantInteger32>(Base)) {
+      // TODO (eholk): creating a new basic block on every memory access is
+      // terrible (see https://goo.gl/Zj7DTr). Try adding a new instruction that
+      // encapsulates this "abort if false" pattern.
+      auto *CheckPassed = Func->makeNode();
+      auto *CheckFailed = getBoundsFailTarget();
+
+      auto *Check = makeVariable(IceType_i1);
+      Control()->appendInst(InstIcmp::create(Func, InstIcmp::Ult, Check, Base,
+                                             Ctx->getConstantInt32(MemSize)));
+      Control()->appendInst(
+          InstBr::create(Func, Check, CheckPassed, CheckFailed));
+
+      *ControlPtr = OperandNode(CheckPassed);
+    }
+
+    Ice::Operand *RealAddr = nullptr;
+    auto MemBase = getWasmMemory();
+    if (!ConstZeroBase) {
+      auto RealAddrV = Func->makeVariable(Ice::getPointerType());
+      Control()->appendInst(InstArithmetic::create(Func, InstArithmetic::Add,
+                                                   RealAddrV, Base, MemBase));
+
+      RealAddr = RealAddrV;
+    } else {
+      RealAddr = MemBase;
+    }
+    return RealAddr;
+  }
+
   template <typename F = std::function<void(Ostream &)>> void log(F Fn) const {
     if (BuildDefs::dump() && (getFlags().getVerbose() & IceV_Wasm)) {
       Fn(Ctx->getStrDump());
       Ctx->getStrDump().flush();
     }
   }
 };
 
 std::unique_ptr<Cfg> WasmTranslator::translateFunction(Zone *Zone,
                                                        FunctionBody &Body) {
   OstreamLocker L1(Ctx);
   auto Func = Cfg::create(Ctx, getNextSequenceNumber());
+  TimerMarker T(TimerStack::TT_wasmGenIce, Func.get());
   Ice::CfgLocalAllocatorScope L2(Func.get());
 
   // TODO(eholk): parse the function signature...
 
   Func->setEntryNode(Func->makeNode());
 
   IceBuilder Builder(Func.get());
   SR_WasmDecoder<OperandNode, IceBuilder> Decoder(Zone, &Builder, Body);
 
   LOG(out << getFlags().getDefaultGlobalPrefix() << "\n");
   Decoder.Decode();
 
   // We don't always know where the incoming branches are in phi nodes, so this
   // function finds them.
   Func->fixPhiNodes();
 
   Func->computeInOutEdges();
 
   return Func;
 }
 
 constexpr SizeT InitialBufferSize = 16 << 10; // 16KB
 
 WasmTranslator::WasmTranslator(GlobalContext *Ctx)
     : Translator(Ctx), Buffer(InitialBufferSize) {}
 
 void WasmTranslator::translate(
     const std::string &IRFilename,
     std::unique_ptr<llvm::DataStreamer> InputStream) {
+  TimerMarker T(TimerStack::TT_wasm, Ctx);
+
   LOG(out << "Initializing v8/wasm stuff..."
           << "\n");
   Zone Zone;
   ZoneScope _(&Zone);
 
   SizeT BytesRead = 0;
   while (true) {
     BytesRead +=
         InputStream->GetBytes(&Buffer[BytesRead], Buffer.size() - BytesRead);
     LOG(out << "Read " << BytesRead << " bytes"
@@ skipping 94 matching lines @@
   Body.module = &ModuleEnv;
 
   LOG(out << "Translating " << IRFilename << "\n");
 
   {
     unique_ptr<VariableDeclarationList> Globals =
         makeUnique<VariableDeclarationList>();
 
     // Global variables, etc go here.
     auto *WasmMemory = VariableDeclaration::createExternal(Globals.get());
-    WasmMemory->setName(Ctx->getGlobalString("WASM_MEMORY"));
+    WasmMemory->setName(Ctx->getGlobalString("WASM_DATA_INIT"));
 
     // Fill in the segments
     SizeT WritePtr = 0;
     for (const auto Seg : Module->data_segments) {
       // fill in gaps with zero.
       if (Seg.dest_addr > WritePtr) {
         WasmMemory->addInitializer(VariableDeclaration::ZeroInitializer::create(
             Globals.get(), Seg.dest_addr - WritePtr));
         WritePtr = Seg.dest_addr;
       }
 
       // 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;
     }
 
     // Save the size of the initialized data in a global variable so the runtime
     // can use it to determine the initial heap break.
     auto *GlobalDataSize = VariableDeclaration::createExternal(Globals.get());
     GlobalDataSize->setName(Ctx->getGlobalString("WASM_DATA_SIZE"));
     GlobalDataSize->addInitializer(VariableDeclaration::DataInitializer::create(
         Globals.get(), reinterpret_cast<const char *>(&WritePtr),
         sizeof(WritePtr)));
 
-    // Pad the rest with zeros
-    SizeT DataSize = Module->min_mem_pages * WASM_PAGE_SIZE;
-    if (WritePtr < DataSize) {
-      WasmMemory->addInitializer(VariableDeclaration::ZeroInitializer::create(
-          Globals.get(), DataSize - WritePtr));
-    }
-
     // Save the number of pages for the runtime
     auto *GlobalNumPages = VariableDeclaration::createExternal(Globals.get());
     GlobalNumPages->setName(Ctx->getGlobalString("WASM_NUM_PAGES"));
     GlobalNumPages->addInitializer(VariableDeclaration::DataInitializer::create(
         Globals.get(), reinterpret_cast<const char *>(&Module->min_mem_pages),
         sizeof(Module->min_mem_pages)));
 
     Globals->push_back(WasmMemory);
     Globals->push_back(GlobalDataSize);
     Globals->push_back(GlobalNumPages);
 
     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.data();
     Body.start = Buffer.data() + Fn.code_start_offset;
     Body.end = Buffer.data() + Fn.code_end_offset;
 
-    auto Func = translateFunction(&Zone, Body);
-    Func->setFunctionName(Ctx->getGlobalString(FnName));
-
+    std::unique_ptr<Cfg> Func = nullptr;
+    {
+      TimerMarker T_func(getContext(), FnName);
+      Func = translateFunction(&Zone, Body);
+      Func->setFunctionName(Ctx->getGlobalString(FnName));
+    }
     Ctx->optQueueBlockingPush(makeUnique<CfgOptWorkItem>(std::move(Func)));
     LOG(out << "done.\n");
   }
 
   return;
 }
 
 #endif // ALLOW_WASM