[Hexagon] Add unit tests for Hexagon Device API

docker/Dockerfile.ci_hexagon

@@ -62,8 +62,7 @@ ENV HEXAGON_SDK_ROOT "/opt/qualcomm/hexagon_sdk"
 ENV CLANG_LLVM_HOME /opt/clang-llvm
 ENV LD_LIBRARY_PATH $LD_LIBRARY_PATH:/opt/clang-llvm/lib
 ENV PATH /opt/clang-llvm/bin:$PATH
-ENV HEXAGON_TOOLCHAIN "${HEXAGON_SDK_ROOT}/tools/HEXAGON_Tools/8.5.08/Tools"
-ENV HEXAGON_GTEST "${HEXAGON_SDK_ROOT}/utils/googletest/gtest"
+ENV HEXAGON_TOOLCHAIN "${HEXAGON_SDK_PATH}/tools/HEXAGON_Tools/8.5.08/Tools"
 
 # sccache
 COPY install/ubuntu_install_sccache.sh /install/ubuntu_install_sccache.sh

src/runtime/hexagon/hexagon_device_api.cc

@@ -55,10 +55,15 @@ void HexagonDeviceAPI::GetAttr(Device dev, DeviceAttrKind kind, TVMRetValue* rv)
 // DataSpace: static allocations for Hexagon
 void* HexagonDeviceAPI::AllocDataSpace(Device dev, int ndim, const int64_t* shape, DLDataType dtype,
                                        Optional<String> mem_scope) {
+  CHECK(shape) << "shape array is null";
+  CHECK(IsValidDevice(dev)) << "dev.device_type: " << dev.device_type;
+
   if (!mem_scope.defined() || mem_scope.value() == "global") {
     return DeviceAPI::AllocDataSpace(dev, ndim, shape, dtype, mem_scope);
   }
 
+  // must be Hexagon device and VTCM scope after this point
+  CHECK_EQ(mem_scope.value(), "global.vtcm");
   CHECK(TVMDeviceExtType(dev.device_type) == kDLHexagon) << "dev.device_type: " << dev.device_type;
 
   size_t typesize = (dtype.bits / 8) * dtype.lanes;
@@ -68,7 +73,9 @@ void* HexagonDeviceAPI::AllocDataSpace(Device dev, int ndim, const int64_t* shap
     alignment = kHexagonAllocAlignment;
   }
 
-  if (ndim == 1) {
+  if (ndim == 0) {
+    return AllocateHexagonBuffer(typesize, alignment, mem_scope);
+  } else if (ndim == 1) {
     size_t nbytes = shape[0] * typesize;
     return AllocateHexagonBuffer(nbytes, alignment, mem_scope);
   } else if (ndim == 2) {
@@ -84,21 +91,18 @@ void* HexagonDeviceAPI::AllocDataSpace(Device dev, int ndim, const int64_t* shap
 
 void* HexagonDeviceAPI::AllocDataSpace(Device dev, size_t nbytes, size_t alignment,
                                        DLDataType type_hint) {
-  // Added kDLCPU since we use hexagon as a sub-target of LLVM which by default maps to kDLCPU;
-  bool is_valid_device = (TVMDeviceExtType(dev.device_type) == kDLHexagon) ||
-                         (DLDeviceType(dev.device_type) == kDLCPU);
-  CHECK(is_valid_device) << "dev.device_type: " << dev.device_type;
+  CHECK(nbytes) << "number of bytes is zero";
+  CHECK(alignment) << "alignment is zero";
+  CHECK(IsValidDevice(dev)) << "dev.device_type: " << dev.device_type;
   if (alignment < kHexagonAllocAlignment) {
     alignment = kHexagonAllocAlignment;
   }
   return AllocateHexagonBuffer(nbytes, alignment, String("global"));
 }
 
 void HexagonDeviceAPI::FreeDataSpace(Device dev, void* ptr) {
-  // Added kDLCPU since we use hexagon as a sub-target of LLVM which by default maps to kDLCPU;
-  bool is_valid_device = (TVMDeviceExtType(dev.device_type) == kDLHexagon) ||
-                         (DLDeviceType(dev.device_type) == kDLCPU);
-  CHECK(is_valid_device) << "dev.device_type: " << dev.device_type;
+  CHECK(ptr) << "buffer pointer is null";
+  CHECK(IsValidDevice(dev)) << "dev.device_type: " << dev.device_type;
   FreeHexagonBuffer(ptr);
 }
 
@@ -109,31 +113,27 @@ struct HexagonWorkspacePool : public WorkspacePool {
 };
 
 void* HexagonDeviceAPI::AllocWorkspace(Device dev, size_t size, DLDataType type_hint) {
-  // Added kDLCPU since we use hexagon as a sub-target of LLVM which by default maps to kDLCPU;
-  bool is_valid_device = (TVMDeviceExtType(dev.device_type) == kDLHexagon) ||
-                         (DLDeviceType(dev.device_type) == kDLCPU);
-  CHECK(is_valid_device) << "dev.device_type: " << dev.device_type;
+  CHECK(IsValidDevice(dev)) << "dev.device_type: " << dev.device_type;
   return dmlc::ThreadLocalStore<HexagonWorkspacePool>::Get()->AllocWorkspace(dev, size);
 }
 
 void HexagonDeviceAPI::FreeWorkspace(Device dev, void* data) {
-  // Added kDLCPU since we use hexagon as a sub-target of LLVM which by default maps to kDLCPU;
-  bool is_valid_device = (TVMDeviceExtType(dev.device_type) == kDLHexagon) ||
-                         (DLDeviceType(dev.device_type) == kDLCPU);
-  CHECK(is_valid_device) << "dev.device_type: " << dev.device_type;
+  CHECK(IsValidDevice(dev)) << "dev.device_type: " << dev.device_type;
   CHECK(hexagon_buffer_map_.count(data) != 0)
       << "Attempt made to free unknown or already freed workspace allocation";
   dmlc::ThreadLocalStore<HexagonWorkspacePool>::Get()->FreeWorkspace(dev, data);
 }
 
 void* HexagonDeviceAPI::AllocVtcmWorkspace(Device dev, int ndim, const int64_t* shape,
                                            DLDataType dtype, Optional<String> mem_scope) {
+  // must be Hexagon device (not CPU)
   CHECK(TVMDeviceExtType(dev.device_type) == kDLHexagon) << "dev.device_type: " << dev.device_type;
   CHECK((ndim == 1 || ndim == 2) && "Hexagon Device API supports only 1d and 2d allocations");
   return AllocDataSpace(dev, ndim, shape, dtype, mem_scope);
 }
 
 void HexagonDeviceAPI::FreeVtcmWorkspace(Device dev, void* ptr) {
+  // must be Hexagon device (not CPU)
   CHECK(TVMDeviceExtType(dev.device_type) == kDLHexagon) << "dev.device_type: " << dev.device_type;
   FreeDataSpace(dev, ptr);
 }

src/runtime/hexagon/hexagon_device_api.h

@@ -138,6 +138,16 @@ class HexagonDeviceAPI final : public DeviceAPI {
     hexagon_buffer_map_.insert({ptr, std::move(buf)});
     return ptr;
   }
+
+  /*! \brief Helper to check if the device type is valid for the Hexagon Device API
+   *  \return Boolean indicating whether the device type is valid
+   */
+  bool IsValidDevice(DLDevice dev) {
+    // Added kDLCPU since we use hexagon as a sub-target of LLVM which by default maps to kDLCPU
+    return (TVMDeviceExtType(dev.device_type) == kDLHexagon) ||
+           (DLDeviceType(dev.device_type) == kDLCPU);
+  }
+
   /*! \brief Helper to free a HexagonBuffer and unregister the result
    *  from the owned buffer map.
    */

tests/cpp-runtime/hexagon/hexagon_device_api_tests.cc

@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <gtest/gtest.h>
+
+#include "../src/runtime/hexagon/hexagon_device_api.h"
+
+using namespace tvm::runtime;
+using namespace tvm::runtime::hexagon;
+
+class HexagonDeviceAPITest : public ::testing::Test {
+ protected:
+  void SetUp() override {
+    hexapi = HexagonDeviceAPI::Global();
+    cpu_dev.device_type = DLDeviceType(kDLCPU);
+    hex_dev.device_type = DLDeviceType(kDLHexagon);
+    invalid_dev.device_type = DLDeviceType(kDLExtDev);
+    int8.bits = 8;
+    int8.code = 0;
+    int8.lanes = 1;
+  }
+  DLDevice cpu_dev;
+  DLDevice hex_dev;
+  DLDevice invalid_dev;
+  DLDataType int8;
+  HexagonDeviceAPI* hexapi;
+  size_t nbytes{256};
+  size_t alignment{64};
+  int64_t shape1d[1]{256};
+  int64_t shape2d[2]{256, 256};
+  int64_t shape3d[3]{256, 256, 256};
+  Optional<String> default_scope;
+  Optional<String> invalid_scope{"invalid"};
+  Optional<String> global_scope{"global"};
+  Optional<String> global_vtcm_scope{"global.vtcm"};
+};
+
+TEST_F(HexagonDeviceAPITest, global) { CHECK(hexapi != nullptr); }
+
+TEST_F(HexagonDeviceAPITest, alloc_free_cpu) {
+  void* buf = hexapi->AllocDataSpace(cpu_dev, nbytes, alignment, int8);
+  CHECK(buf != nullptr);
+  hexapi->FreeDataSpace(cpu_dev, buf);
+}
+
+TEST_F(HexagonDeviceAPITest, alloc_free_hex) {
+  void* buf = hexapi->AllocDataSpace(hex_dev, nbytes, alignment, int8);
+  CHECK(buf != nullptr);
+  hexapi->FreeDataSpace(hex_dev, buf);
+}
+
+TEST_F(HexagonDeviceAPITest, alloc_errors) {
+  // invalid device
+  EXPECT_THROW(hexapi->AllocDataSpace(invalid_dev, nbytes, alignment, int8), InternalError);
+  // 0 size
+  EXPECT_THROW(hexapi->AllocDataSpace(hex_dev, 0, alignment, int8), InternalError);
+  // 0 alignment
+  EXPECT_THROW(hexapi->AllocDataSpace(hex_dev, nbytes, 0, int8), InternalError);
+}
+
+TEST_F(HexagonDeviceAPITest, free_errors) {
+  void* buf = hexapi->AllocDataSpace(hex_dev, nbytes, alignment, int8);
+
+  // invalid device
+  EXPECT_THROW(hexapi->FreeDataSpace(invalid_dev, buf), InternalError);
+  // invalid pointer
+  EXPECT_THROW(hexapi->FreeDataSpace(hex_dev, &buf), InternalError);
+  // nullptr
+  EXPECT_THROW(hexapi->FreeDataSpace(hex_dev, nullptr), InternalError);
+  // double free
+  hexapi->FreeDataSpace(hex_dev, buf);
+  EXPECT_THROW(hexapi->FreeDataSpace(hex_dev, buf), InternalError);
+}
+
+TEST_F(HexagonDeviceAPITest, allocnd_free_cpu) {
+  void* buf = hexapi->AllocDataSpace(cpu_dev, 3, shape3d, int8, global_scope);
+  CHECK(buf != nullptr);
+  hexapi->FreeDataSpace(cpu_dev, buf);
+}
+
+TEST_F(HexagonDeviceAPITest, allocnd_free_hex) {
+  void* buf = hexapi->AllocDataSpace(hex_dev, 3, shape3d, int8, global_scope);
+  CHECK(buf != nullptr);
+  hexapi->FreeDataSpace(hex_dev, buf);
+}
+
+TEST_F(HexagonDeviceAPITest, allocnd_free_hex_vtcm) {
+  void* buf1d = hexapi->AllocDataSpace(hex_dev, 1, shape1d, int8, global_vtcm_scope);
+  CHECK(buf1d != nullptr);
+  hexapi->FreeDataSpace(hex_dev, buf1d);
+
+  void* buf2d = hexapi->AllocDataSpace(hex_dev, 2, shape2d, int8, global_vtcm_scope);
+  CHECK(buf2d != nullptr);
+  hexapi->FreeDataSpace(hex_dev, buf2d);
+}
+
+TEST_F(HexagonDeviceAPITest, allocnd_erros) {
+  // invalid device
+  EXPECT_THROW(hexapi->AllocDataSpace(invalid_dev, 2, shape2d, int8, global_vtcm_scope),
+               InternalError);
+
+  // Hexagon VTCM allocations must have 0 (scalar) 1 or 2 dimensions
+  EXPECT_THROW(hexapi->AllocDataSpace(hex_dev, 3, shape3d, int8, global_vtcm_scope), InternalError);
+
+  // null shape
+  EXPECT_THROW(hexapi->AllocDataSpace(hex_dev, 2, nullptr, int8, global_vtcm_scope), InternalError);
+
+  // null shape
+  EXPECT_THROW(hexapi->AllocDataSpace(hex_dev, 2, shape2d, int8, invalid_scope), InternalError);
+
+  // cpu & global.vtcm scope
+  EXPECT_THROW(hexapi->AllocDataSpace(cpu_dev, 2, shape2d, int8, global_vtcm_scope), InternalError);
+}
+
+TEST_F(HexagonDeviceAPITest, alloc_scalar) {
+  void* cpuscalar = hexapi->AllocDataSpace(cpu_dev, 0, new int64_t, int8, global_scope);
+  CHECK(cpuscalar != nullptr);
+
+  void* hexscalar = hexapi->AllocDataSpace(hex_dev, 0, new int64_t, int8, global_vtcm_scope);
+  CHECK(hexscalar != nullptr);
+}
+
+// alloc and free of the same buffer on different devices should throw
+// but it currently works with no error
+// hexagon and cpu device types may merge long term which would make this test case moot
+// disabling this test case, for now
+// TODO(HWE): Re-enable or delete this test case once we land on device type strategy
+TEST_F(HexagonDeviceAPITest, DISABLED_alloc_free_diff_dev) {
+  void* buf = hexapi->AllocDataSpace(hex_dev, nbytes, alignment, int8);
+  CHECK(buf != nullptr);
+  EXPECT_THROW(hexapi->FreeDataSpace(cpu_dev, buf), InternalError);
+}

tests/python/contrib/test_hexagon/test_run_unit_tests.py

@@ -27,16 +27,12 @@
 # for example to run all "foo" tests twice and observe gtest output run
 # pytest -sv <this file> --gtests_args="--gtest_filter=*foo* --gtest_repeat=2"
 @requires_hexagon_toolchain
-@pytest.mark.skipif(
-    os.environ.get("HEXAGON_GTEST") == None,
-    reason="Test requires environment variable HEXAGON_GTEST set with a path to a Hexagon gtest version normally located at /path/to/hexagon/sdk/utils/googletest/gtest",
-)
 def test_run_unit_tests(hexagon_session, gtest_args):
     try:
         func = hexagon_session._rpc.get_function("hexagon.run_unit_tests")
     except:
         print(
-            "Test requires TVM Runtime to be built with a Hexagon gtest version using Hexagon API cmake flag -DUSE_HEXAGON_GTEST=${HEXAGON_GTEST}"
+            "This test requires TVM Runtime to be built with a Hexagon gtest version using Hexagon API cmake flag -DUSE_HEXAGON_GTEST=/path/to/hexagon/sdk/utils/googletest/gtest"
         )
         raise
 

tests/scripts/task_build_hexagon_api.sh

@@ -37,16 +37,13 @@ cd build
 output_binary_directory=$(realpath ${PWD}/../../../build/hexagon_api_output)
 rm -rf ${output_binary_directory}
 
-# should be removed after Hexagon Docker update
-export HEXAGON_GTEST="${HEXAGON_SDK_PATH}/utils/googletest/gtest"
-
 cmake -DANDROID_ABI=arm64-v8a \
     -DANDROID_PLATFORM=android-28 \
     -DUSE_ANDROID_TOOLCHAIN="${ANDROID_NDK_HOME}/build/cmake/android.toolchain.cmake" \
     -DUSE_HEXAGON_ARCH=v68 \
     -DUSE_HEXAGON_SDK="${HEXAGON_SDK_PATH}" \
     -DUSE_HEXAGON_TOOLCHAIN="${HEXAGON_TOOLCHAIN}" \
     -DUSE_OUTPUT_BINARY_DIR="${output_binary_directory}" \
-    -DUSE_HEXAGON_GTEST="${HEXAGON_GTEST}" ..
+    -DUSE_HEXAGON_GTEST="${HEXAGON_SDK_PATH}/utils/googletest/gtest" ..
 
 make -j$(nproc)

tests/scripts/task_python_hexagon.sh

@@ -43,9 +43,6 @@ if [[ "${device_serial}" == "simulator" ]]; then
     export HEXAGON_SDK_ROOT=${HEXAGON_SDK_PATH}
 fi
 
-# should be removed after Hexagon Docker update
-export HEXAGON_GTEST="${HEXAGON_SDK_PATH}/utils/googletest/gtest"
-
 export ANDROID_SERIAL_NUMBER=${device_serial}
 run_pytest ctypes python-contrib-hexagon tests/python/contrib/test_hexagon