checkpoint in bughunting

frontends/PyCDE/integration_test/esitester.py

@@ -249,14 +249,15 @@ class EsiTesterTop(Module):
 
   @generator
   def construct(ports):
-    PrintfExample(clk=ports.clk, rst=ports.rst)
-    ReadMem(32)(appid=esi.AppID("readmem", 32), clk=ports.clk, rst=ports.rst)
-    ReadMem(64)(appid=esi.AppID("readmem", 64), clk=ports.clk, rst=ports.rst)
-    ReadMem(96)(appid=esi.AppID("readmem", 96), clk=ports.clk, rst=ports.rst)
-    WriteMem(32)(appid=esi.AppID("writemem", 32), clk=ports.clk, rst=ports.rst)
-    WriteMem(64)(appid=esi.AppID("writemem", 64), clk=ports.clk, rst=ports.rst)
-    WriteMem(96)(appid=esi.AppID("writemem", 96), clk=ports.clk, rst=ports.rst)
-    for width in [32, 64, 128, 256, 384, 504, 512]:
+    # PrintfExample(clk=ports.clk, rst=ports.rst)
+    for width in [32, 64, 96, 128, 256, 384, 504, 512]:
+      # for width in [504, 512]:
+      ReadMem(width)(appid=esi.AppID("readmem", width),
+                     clk=ports.clk,
+                     rst=ports.rst)
+      WriteMem(width)(appid=esi.AppID("writemem", width),
+                      clk=ports.clk,
+                      rst=ports.rst)
       ToHostDMATest(width)(appid=esi.AppID("tohostdmatest", width),
                            clk=ports.clk,
                            rst=ports.rst)

frontends/PyCDE/src/pycde/bsp/common.py

@@ -513,7 +513,10 @@ def build(ports):
         # to complete the transmission.
         num_chunks = TaggedWriteGearboxImpl.num_chunks
         num_chunks_idx_bitwidth = clog2(num_chunks)
-        padding_numbits = output_bitwidth - (input_bitwidth % output_bitwidth)
+        if input_bitwidth % output_bitwidth == 0:
+          padding_numbits = 0
+        else:
+          padding_numbits = output_bitwidth - (input_bitwidth % output_bitwidth)
         assert padding_numbits % 8 == 0, "Padding must be a multiple of 8."
         client_data_padded = BitsSignal.concat(
             [Bits(padding_numbits)(0), client_data])

frontends/PyCDE/src/pycde/bsp/xrt.py

@@ -383,7 +383,7 @@ def construct_hostmem(ports):
       ports.m_axi_gmem_WVALID = data_req_valid
       ports.m_axi_gmem_WDATA = data_req_data.data
 
-      wstrb_lookup_table_len = HostMemDataWidthBytes
+      wstrb_lookup_table_len = HostMemDataWidthBytes + 1
       wstrb_lookup_table = Array(Bits(HostMemDataWidthBytes),
                                  wstrb_lookup_table_len)([
                                      Bits(HostMemDataWidthBytes)(2**vb - 1)

frontends/PyCDE/src/pycde/esi.py

@@ -275,7 +275,6 @@ def emit_engine(self,
                   engine: EngineModule,
                   details: Dict[str, object] = None):
     """Emit and return an engine record."""
-    details = optional_dict_to_dict_attr(details)
     with get_user_loc(), ir.InsertionPoint(self._rec):
       return EngineServiceRecord(engine, details)
 

frontends/PyCDE/src/pycde/types.py

@@ -481,9 +481,29 @@ def _get_value_class(self):
     return BitsSignal
 
   def _from_obj(self, x: int, alias: typing.Optional[TypeAlias] = None):
+    # The MLIR python bindings don't support IntegerAttributes with unsigned
+    # 64-bit values since the bindings accept signed integers so an unsigned
+    # 64-bit integer overflows that.
+    # https://github.com/llvm/llvm-project/issues/128072
+    # So we need to work around that here using concats.
+    # TODO: Adapt this to UInt and SInt.
     from .dialects import hw
     self._from_obj_check(x)
     circt_type = self if alias is None else alias
+    if x.bit_length() > 63:
+      # Split the int into an array of 32-bit ints.
+      chunks = [(x >> i) & 0xFFFFFFFF for i in range(0, x.bit_length(), 32)]
+      last_bitwidth = self.bitwidth % 32
+      if last_bitwidth == 0:
+        last_bitwidth = 32
+      chunk_consts = [
+          hw.ConstantOp(ir.IntegerType.get_signless(32), c) for c in chunks[:-1]
+      ]
+      chunk_consts.append(
+          hw.ConstantOp(ir.IntegerType.get_signless(last_bitwidth), chunks[-1]))
+
+      from .signals import BitsSignal
+      return BitsSignal.concat(chunk_consts).bitcast(circt_type)
     return hw.ConstantOp(circt_type, x)
 
 

lib/Dialect/ESI/runtime/cpp/lib/Design.cpp

@@ -76,6 +76,8 @@ BundlePort *HWModule::resolvePort(const AppIDPath &path,
   instPath.pop_back();
   const HWModule *hwmodule = resolveInst(instPath, lastLookup);
   lastLookup.push_back(portID);
+  if (!hwmodule)
+    return nullptr;
   const auto &ports = hwmodule->getPorts();
   const auto &portIter = ports.find(portID);
   if (portIter == ports.end())

lib/Dialect/ESI/runtime/cpp/lib/Engines.cpp

@@ -64,8 +64,6 @@ class OneItemBuffersToHostReadPort : public ReadChannelPort {
 public:
   /// Offset into the MMIO space to which the buffer pointer is written.
   static constexpr size_t BufferPtrOffset = 8;
-  /// Offset into the buffer where the valid byte is stored.
-  static constexpr size_t ValidDataOffset = 0;
 
   OneItemBuffersToHostReadPort(const Type *type, OneItemBuffersToHost *engine)
       : ReadChannelPort(type), engine(engine) {
@@ -80,6 +78,7 @@ class OneItemBuffersToHostReadPort : public ReadChannelPort {
   size_t bufferSize;
   OneItemBuffersToHost *engine;
   std::unique_ptr<services::HostMem::HostMemRegion> buffer;
+  uint64_t pollCount = 0;
 };
 
 class OneItemBuffersToHost : public Engine {
@@ -155,13 +154,24 @@ void OneItemBuffersToHostReadPort::writeBufferPtr() {
 
 void OneItemBuffersToHostReadPort::connectImpl(std::optional<unsigned>) {
   engine->connect();
-  buffer = engine->hostMem->allocate(bufferSize, {});
+  buffer = engine->hostMem->allocate(128, {});
   memset(buffer->getPtr(), 0, bufferSize);
   writeBufferPtr();
 }
 
 bool OneItemBuffersToHostReadPort::pollImpl() {
   uint8_t *bufferData = reinterpret_cast<uint8_t *>(buffer->getPtr());
+  if (pollCount++ % 100000 == 0) {
+    engine->conn.getLogger().debug(
+        [this, bufferData](
+            std::string &subsystem, std::string &msg,
+            std::unique_ptr<std::map<std::string, std::any>> &details) {
+          subsystem = "OneItemBuffersToHost";
+          msg = "Polling buffer contents 0x";
+          for (size_t i = 0; i < 128; ++i)
+            msg += toHex(bufferData[i]);
+        });
+  }
   if (bufferData[bufferSize - 1] == 0)
     return false;
 

lib/Dialect/ESI/runtime/cpp/lib/Logging.cpp

@@ -41,19 +41,19 @@ void StreamLogger::logImpl(Level level, const std::string &subsystem,
 
   switch (level) {
   case Level::Error:
-    os << "[ERROR] ";
+    os << "[  ERROR] ";
     indentSpaces = 8;
     break;
   case Level::Warning:
     os << "[WARNING] ";
     indentSpaces = 10;
     break;
   case Level::Info:
-    os << "[INFO] ";
+    os << "[  INFO] ";
     indentSpaces = 7;
     break;
   case Level::Debug:
-    os << "[DEBUG] ";
+    os << "[ DEBUG] ";
     indentSpaces = 8;
     break;
   }
@@ -64,11 +64,12 @@ void StreamLogger::logImpl(Level level, const std::string &subsystem,
   }
   os << msg << std::endl;
 
-  if (!details)
-    return;
-  std::string indent(indentSpaces, ' ');
-  for (const auto &detail : *details)
-    os << indent << detail.first << ": " << toString(detail.second) << "\n";
+  if (details) {
+    std::string indent(indentSpaces, ' ');
+    for (const auto &detail : *details)
+      os << indent << detail.first << ": " << toString(detail.second) << "\n";
+  }
+  os.flush();
 }
 
 std::string esi::toString(const std::any &value) {

lib/Dialect/ESI/runtime/cpp/lib/backends/Cosim.cpp

@@ -226,10 +226,11 @@ class ReadCosimChannelPort
     : public ReadChannelPort,
       public grpc::ClientReadReactor<esi::cosim::Message> {
 public:
-  ReadCosimChannelPort(ChannelServer::Stub *rpcClient, const ChannelDesc &desc,
+  ReadCosimChannelPort(AcceleratorConnection &conn,
+                       ChannelServer::Stub *rpcClient, const ChannelDesc &desc,
                        const Type *type, std::string name)
-      : ReadChannelPort(type), rpcClient(rpcClient), desc(desc), name(name),
-        context(nullptr) {}
+      : ReadChannelPort(type), conn(conn), rpcClient(rpcClient), desc(desc),
+        name(name), context(nullptr) {}
   virtual ~ReadCosimChannelPort() { disconnect(); }
 
   void connectImpl(std::optional<unsigned> bufferSize) override {
@@ -272,6 +273,8 @@ class ReadCosimChannelPort
 
   /// Disconnect this channel from the server.
   void disconnect() override {
+    Logger &logger = conn.getLogger();
+    logger.debug("cosim_read", "Disconnecting channel " + name);
     if (!context)
       return;
     context->TryCancel();
@@ -280,6 +283,7 @@ class ReadCosimChannelPort
   }
 
 protected:
+  AcceleratorConnection &conn;
   ChannelServer::Stub *rpcClient;
   /// The channel description as provided by the server.
   ChannelDesc desc;
@@ -335,7 +339,7 @@ class CosimMMIO : public MMIO {
     cmdArgPort = std::make_unique<WriteCosimChannelPort>(
         rpcClient->stub.get(), cmdArg, cmdType, "__cosim_mmio_read_write.arg");
     cmdRespPort = std::make_unique<ReadCosimChannelPort>(
-        rpcClient->stub.get(), cmdResp, i64Type,
+        conn, rpcClient->stub.get(), cmdResp, i64Type,
         "__cosim_mmio_read_write.result");
     auto *bundleType = new BundleType(
         "cosimMMIO", {{"arg", BundleType::Direction::To, cmdType},
@@ -359,10 +363,24 @@ class CosimMMIO : public MMIO {
     auto arg = MessageData::from(cmd);
     std::future<MessageData> result = cmdMMIO->call(arg);
     result.wait();
-    return *result.get().as<uint64_t>();
+    uint64_t ret = *result.get().as<uint64_t>();
+    conn.getLogger().debug(
+        [addr, ret](std::string &subsystem, std::string &msg,
+                    std::unique_ptr<std::map<std::string, std::any>> &details) {
+          subsystem = "cosim_mmio";
+          msg = "MMIO[0x" + toHex(addr) + "] = 0x" + toHex(ret);
+        });
+    return ret;
   }
 
   void write(uint32_t addr, uint64_t data) override {
+    conn.getLogger().debug(
+        [addr,
+         data](std::string &subsystem, std::string &msg,
+               std::unique_ptr<std::map<std::string, std::any>> &details) {
+          subsystem = "cosim_mmio";
+          msg = "MMIO[0x" + toHex(addr) + "] <- 0x" + toHex(data);
+        });
     MMIOCmd cmd{.data = data, .offset = addr, .write = true};
     auto arg = MessageData::from(cmd);
     std::future<MessageData> result = cmdMMIO->call(arg);
@@ -415,6 +433,9 @@ class CosimHostMem : public HostMem {
     // We have to locate the channels ourselves since this service might be used
     // to retrieve the manifest.
 
+    if (writeRespPort)
+      return;
+
     // TODO: The types here are WRONG. They need to be wrapped in Channels! Fix
     // this in a subsequent PR.
 
@@ -440,7 +461,7 @@ class CosimHostMem : public HostMem {
         rpcClient->stub.get(), readResp, readRespType,
         "__cosim_hostmem_read_resp.data");
     readReqPort = std::make_unique<ReadCosimChannelPort>(
-        rpcClient->stub.get(), readArg, readReqType,
+        conn, rpcClient->stub.get(), readArg, readReqType,
         "__cosim_hostmem_read_req.data");
     readReqPort->connect(
         [this](const MessageData &req) { return serviceRead(req); });
@@ -464,7 +485,7 @@ class CosimHostMem : public HostMem {
         rpcClient->stub.get(), writeResp, writeRespType,
         "__cosim_hostmem_write.result");
     writeReqPort = std::make_unique<ReadCosimChannelPort>(
-        rpcClient->stub.get(), writeArg, writeReqType,
+        conn, rpcClient->stub.get(), writeArg, writeReqType,
         "__cosim_hostmem_write.arg");
     auto *bundleType = new BundleType(
         "cosimHostMem",
@@ -643,7 +664,7 @@ CosimEngine::createPort(AppIDPath idPath, const std::string &channelName,
         conn.rpcClient->stub.get(), chDesc, type, fullChannelName);
   else
     port = std::make_unique<ReadCosimChannelPort>(
-        conn.rpcClient->stub.get(), chDesc, type, fullChannelName);
+        conn, conn.rpcClient->stub.get(), chDesc, type, fullChannelName);
   return port;
 }
 

lib/Dialect/ESI/runtime/cpp/lib/backends/Xrt.cpp

@@ -101,11 +101,27 @@ class XrtMMIO : public MMIO {
       : MMIO(conn, clients), ip(ip) {}
 
   uint64_t read(uint32_t addr) const override {
+    Logger &logger = conn.getLogger();
     auto lo = static_cast<uint64_t>(ip.read_register(addr));
     auto hi = static_cast<uint64_t>(ip.read_register(addr + 0x4));
-    return (hi << 32) | lo;
+    uint64_t ret = (hi << 32) | lo;
+    logger.debug(
+        [addr, ret](std::string &subsystem, std::string &msg,
+                    std::unique_ptr<std::map<std::string, std::any>> &details) {
+          subsystem = "xrt_mmio";
+          msg = "MMIO[0x" + toHex(addr) + "] = 0x" + toHex(ret);
+        });
+    return ret;
   }
   void write(uint32_t addr, uint64_t data) override {
+    Logger &logger = conn.getLogger();
+    logger.debug(
+        [addr,
+         data](std::string &subsystem, std::string &msg,
+               std::unique_ptr<std::map<std::string, std::any>> &details) {
+          subsystem = "xrt_mmio";
+          msg = "MMIO[0x" + toHex(addr) + "] <- 0x" + toHex(data);
+        });
     ip.write_register(addr, data);
     ip.write_register(addr + 0x4, data >> 32);
   }
@@ -120,7 +136,7 @@ namespace {
 class XrtHostMem : public HostMem {
 public:
   XrtHostMem(XrtAccelerator &conn, ::xrt::device &device, int32_t memoryGroup)
-      : HostMem(conn), device(device), memoryGroup(memoryGroup){};
+      : HostMem(conn), device(device), memoryGroup(memoryGroup) {};
 
   struct XrtHostMemRegion : public HostMemRegion {
     XrtHostMemRegion(::xrt::device &device, std::size_t size,