[GitHub] [incubator-tvm-vta] liangfu commented on a change in pull request #9: [Hardware][OpenCL] Intelfocl support

[GitBox <gi...@apache.org>]

liangfu commented on a change in pull request #9:
URL: https://github.com/apache/incubator-tvm-vta/pull/9#discussion_r456229024



##########
File path: hardware/intelfocl/src/vta.cl
##########
@@ -0,0 +1,341 @@
+#pragma OPENCL EXTENSION cl_intel_channels: enable

Review comment:
       Just curious, as I remember this PR is going to work for ultra96, does this extension work for Xilinx toolchain to compile the hardware design? I might misunderstood somewhere.

##########
File path: src/intelfocl/intelfocl_device.cc
##########
@@ -0,0 +1,185 @@
+#include <numeric>
+#include <dmlc/logging.h>
+#include <vta/hw_spec.h>
+#include "intelfocl_device.h"
+#include "aoclutils/aocl_utils.h"
+
+#define CL_STATUS_SUCCESS(x) ((x) == CL_SUCCESS)
+
+void cleanup() {}
+
+int IntelFOCLDevice::init(size_t mem_size, std::string aocx_file)
+{
+    cl_int status;

Review comment:
       Please ensure the source code pass cpplint rules. Specifically, we use 2 spaces for indentation.

##########
File path: config/intelfocl_sample.json
##########
@@ -0,0 +1,13 @@
+{
+  "TARGET" : "intelfocl",
+  "HW_VER" : "0.0.1",

Review comment:
       We might need to update this version number, since the HW ISA has changed?

##########
File path: config/intelfocl_sample.json
##########
@@ -0,0 +1,13 @@
+{
+  "TARGET" : "intelfocl",
+  "HW_VER" : "0.0.1",
+  "LOG_INP_WIDTH" : 3,

Review comment:
       Since the types are hard coded as 
   ```c
   typedef char            inp_T;
   typedef char            wgt_T;
   ```
   , these config variables (e.g. `LOG_INP_WIDTH`) wouldn't actually reflect the hardware design, right?

##########
File path: src/intelfocl/intelfocl_device.cc
##########
@@ -0,0 +1,185 @@
+#include <numeric>
+#include <dmlc/logging.h>
+#include <vta/hw_spec.h>
+#include "intelfocl_device.h"
+#include "aoclutils/aocl_utils.h"
+
+#define CL_STATUS_SUCCESS(x) ((x) == CL_SUCCESS)
+
+void cleanup() {}

Review comment:
       actually clean up?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org