[RUNTIME] Initial implementation of Hexagon runtime support #3163

kparzysz-quic · 2019-05-09T15:46:04Z

No description provided.

junrushao · 2019-05-09T18:45:11Z

Is this ready for review? I saw a lot of codes commented out

kparzysz-quic · 2019-05-09T18:52:22Z

The commented out parts are mostly debugging code. This is ready for review.

kparzysz-quic · 2019-05-09T18:58:53Z

I will be pushing some changes soon to pacify the cpplint checker.

CMakeLists.txt

src/runtime/hexagon/hexagon_device_api.cc

tqchen · 2019-05-10T03:41:50Z

src/runtime/hexagon/hexagon_device_sim.cc

@@ -0,0 +1,366 @@
+/*!
+ * Copyright 2018-2019 Qualcomm Innovation Center, Inc.


We are in the process of migrating to ASF, and as a future convention we do not keep copyright messages because the code will be licensed to ASF, please check if it is OK.

I will bring it to the attention of the right people.

This may take a bit of time to figure out. If we keep the copyright notices, will it block this PR?

Likely it will be a blocker, but we can first keep reviewing and make sure things are in shape. The code can exist in your repo for now

Will it be acceptable if we remove this single line (that says "Copyright..."), and leave everything else unchanged? We need to have a better idea of what's acceptable before we discuss it with the legal teams.

Yes, that is exactly what I asked for, see https://www.apache.org/legal/src-headers.html
Sorry for the confusion, I meant to remove the single line that says "Copyright.."

Thanks for the clarification.

License header should be

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.

What does that mean exactly?

There is this, plus an extra paragraph in our license. You said you wanted us to remove the copyright line, and that was it. What is the intent of this comment in the light of what had been written before?

Correction: the first paragraph is different. Is this what you are referring to?

src/runtime/hexagon/hexagon_wrapper_dl.h

src/runtime/hexagon/hexagon_module.h

src/runtime/hexagon/hexagon_device_target.h

src/runtime/hexagon/hexagon_module.cc

src/runtime/hexagon/hexagon_device_sim.cc

tqchen · 2019-05-10T04:03:44Z

Some quick comments. Here is a summary:

Style issues(follow google c style)
- Use CHECK_XX style instead of assert
- Use CamelCase for functions
- Add variable_with_underscore_ for class members.

The second things that need some elaboration(perhaps in the comment as well are the level of abstractions introduced.

At the highest level DeviceAPI and Module
hexagon::Device appears to be a low-level device abstraction for hexagon
There seems to be even a LowLevlAPI(I have not checked the code carefully) that contains
- WriteVirtualMemory
- ReadVirtialMemory
- LookupSymbol
- RunSymbol
- Seems similar to LowlevelDeviceAPI of uTVM [RFC][μTVM] Bringing TVM to Bare-Metal Devices #2563

Likely we could only introduce two levels of abstractions. Besides the highest level, we only need another one that is a common denominator of the two.

We also can hide implementations. Specifically, the implementation of hexagon::Device's subclass does not have to appear in the header.

Here is one possible proposal of organization:

src/runtime/hexagon/sim/sim_device.cc
src/runtime/hexagon/frpc/frpc_device.cc
src/runtime/hexagon/device/device_lib.cc (device side library for driving the code.)

Depending on the compilation flag, we only link sim or frpc. Both of them will implement hexagon::Device::Create which creates the hexagon::Device, or the corresponding appropriate subclass.

The above comment is based on my first quick pass over the code, so it might be inaccurate, would love to continue discussion as well.

At the same time, as in uTVM, we want to leave doors open to implement lazy execution. This will likely mean push the tasks as a linked list argument into the argument section, then ask the device side to execute everything in the list. This does not have to be implemented in this PR, but would be a good thing to keep in mind to make sure the design is future compatible

FrozenGene · 2019-05-10T04:07:22Z

src/runtime/hexagon/fastrpc/tvm_hexagon_remote.h

+};
+typedef unsigned int tvm_hexagon_remote_handle_t;
+typedef uint64 tvm_hexagon_remote_scalar_t;
+__QAIC_HEADER_EXPORT int __QAIC_HEADER(tvm_hexagon_remote_map_buffer_to_dsp)(


could we also upload tvm_hexagon_remote_map_buffer_to_dsp definition? i.e. tvm_hexagon_remote_map_buffer_to_dsp_stub.c / tvm_hexagon_remote_map_buffer_to_dsp_skel.c / tvm_hexagon_remote_map_buffer_to_dsp.il`

FrozenGene · 2019-05-10T04:08:19Z

src/runtime/hexagon/fastrpc/tvm_hexagon_remote.h

+#ifndef tvm_hexagon_remote_URI
+#define tvm_hexagon_remote_URI                                            \
+  "file:///"                                                              \
+  "libtvm_hexagon_remote_skel.so?tvm_hexagon_remote_skel_handle_invoke&_" \


libtvm_hexagon_remote_skel.so is not in HexagonSDK, we build it. Could we also upload it?

I'll upload the sources next week. You could use them to build the skel.so and the stub.so.

There is a bit of delay. Hopefully I'll get that done next week.

CMakeLists.txt

FrozenGene · 2019-05-10T04:17:18Z

One common comment:
Currently, we only support CDSP. However, SnapDragon 820 is very popular in industry. 820 doesn't have CDSP and can not select. It only could go to ADSP. Currently, in this PR, we use domain_channel_handle to detect and set data.domain = CDSP_DOMAIN_ID . However, if we are in 820, we could just simply go to void* mem = rpcmem_alloc(RPCMEM_HEAP, RPCMEM_DEFAULT_FLAGS, size); For example:

# ifdef USE_CDSP
if (domain_channel_handle == AEE_EUNKNOWN) {
    if (remote_session_control) {
      remote_rpc_control_unsigned_module data;
      data.enable = 1;
      data.domain = CDSP_DOMAIN_ID;
      int rc = remote_session_control(DSPRPC_CONTROL_UNSIGNED_MODULE, &data,
                                      sizeof(data));
      LOG_TVM(ANDROID_LOG_DEBUG, "TVM",
              "HexagonTarget %s: remote_session_control rc=%d for unsigned PD",
              __func__, rc);
    } else {
      LOG_TVM(ANDROID_LOG_DEBUG, "TVM",
              "HexagonTarget %s: No unsigned PD support available", __func__);
    }
    int rc = tvm_hexagon_remote_open(tvm_hexagon_remote_URI "&_dom=cdsp",
                                     &domain_channel_handle);
    if (rc != AEE_SUCCESS) {
      LOG_TVM(ANDROID_LOG_ERROR, "TVM",
              "HexagonTarget %s:Failed to open channel rc=0x%x", __func__, rc);
      return nullptr;
    }
  }
#endif 
  void* mem = rpcmem_alloc(RPCMEM_HEAP, RPCMEM_DEFAULT_FLAGS, size);
  if (mem == nullptr) {
    LOG_TVM(ANDROID_LOG_ERROR, "TVM",
            "HexagonTarget %sMem alloc failed for size=0x%x alignment=0x%x",
            __func__, size, align);
  }
#if USE_CDSP
  int mem_fd = rpcmem_to_fd(mem);
  uint64_t dsp_va = 0;

  int rc = tvm_hexagon_remote_map_buffer_to_dsp(
      domain_channel_handle, mem_fd, static_cast<int>(size),
      reinterpret_cast<uint64*>(&dsp_va));
  if (rc != AEE_SUCCESS) {
    LOG_TVM(
        ANDROID_LOG_ERROR, "TVM",
        "HexagonTarget %s: buffer mapping to dsp failed for fd=0x%x rc=0x%x",
        __func__, mem_fd, rc);
    return nullptr;
  }

  // LOG_TVM(ANDROID_LOG_ERROR, "TVM",
  //  "Address from remote_map_buffer :0x%x", dsp_va);
  dsp_address_mapping.insert(
      std::make_pair(dsp_va, reinterpret_cast<uint64_t>(mem)));
  dsp_size_mapping.insert(std::make_pair(dsp_va, static_cast<uint64_t>(size)));
  // LOG_TVM(ANDROID_LOG_DEBUG, "TVM",
  //  "HexagonTarget %s: dsp_va:0x%x apps_va:0x%x mem_fd=0x%x",
  //  __func__, static_cast<uint64_t>(dsp_va),
  //  reinterpret_cast<uint64_t>(mem), mem_fd);
  return reinterpret_cast<void*>(dsp_va);
#endif
  return reinterpret_cast<void*>(mem)
}

Could we consider supporting it?

Secondly, in fact, in HexagonSDK, we have UbuntuARM, and we could also support Linux and the code is also very similar. Could we have plan to support it?

junrushao

Some high-level comments:

Avoid hard-coding symbols when possible. If we have to dlopen something, it might be a good idea to have a clear message asking users to properly set some path.
Seeing some code that is commented out. Let's remove them if they are truly useless.
As Tianqi already mentioned, try avoiding asserts and std::cerr.

CMakeLists.txt

junrushao · 2019-05-10T04:34:07Z

src/runtime/hexagon/fastrpc/tvm_hexagon_remote.h

+// qidl copyright
+// qidl nested=false
+#include "AEEStdDef.h"
+#include "android_Release_aarch64/remote.h"


Do we have preference to use <> instead of quotes?

Is that a question for me? I don't know. You tell me...

src/runtime/hexagon/hexagon_device_api.cc

src/runtime/hexagon/hexagon_device_sim.cc

src/runtime/hexagon/hexagon_wrapper_dl.cc

src/runtime/hexagon/hexagon_module.cc

kparzysz-quic · 2019-05-10T19:08:43Z

I hope I have fixed all issues related to coding conventions. I'd like to get that out of the way before making any other changes.

kparzysz-quic · 2019-05-10T19:27:10Z

The second things that need some elaboration(perhaps in the comment as well are the level of abstractions introduced.

At the highest level DeviceAPI and Module

hexagon::Device appears to be a low-level device abstraction for hexagon

There seems to be even a LowLevlAPI(I have not checked the code carefully) that contains

WriteVirtualMemory

ReadVirtialMemory

LookupSymbol

RunSymbol

Seems similar to LowlevelDeviceAPI of uTVM [RFC][μTVM] Bringing TVM to Bare-Metal Devices #2563

Yes, it does. Additionally, what we need is Alloc/Free (for memory) and Load/Unload (for shared objects). With that we could switch to LowLevelDeviceAPI (except that I haven't found in the sources).

Here is one possible proposal of organization:

src/runtime/hexagon/sim/sim_device.cc

src/runtime/hexagon/frpc/frpc_device.cc

src/runtime/hexagon/device/device_lib.cc (device side library for driving the code.)

Depending on the compilation flag, we only link sim or frpc. Both of them will implement hexagon::Device::Create which creates the hexagon::Device, or the corresponding appropriate subclass.

This approach requires that both sides (sim and frpc) define the same "factory" function. This means that they can never be built together. I'm not sure if that's good or bad, but we need to think about long term consequences of this choice.

tqchen · 2019-05-11T00:20:31Z

I think it is fine to assume that we will either build simulator or frpc. We can also have two separate factory functions that allow us to link both.

As I mentioned in the last post. I think we should reduce the level of abstractions into two:

DeviceAPI, Module(tvm's original layer)
LowLevelDeviceAPI, (remove the intermediate hexagon::Device)

The reason is that we are essentially implementing a "driver" based on the low-level device interface. If the low-level interface is somewhat common between the simulator and real hardware, then we should only expose the LowLevelDeviceAPI.

src/runtime/hexagon/hexagon_module.cc

FrozenGene · 2019-05-13T16:34:02Z

include/tvm/runtime/device_api.h


 /*! \brief Number of bytes each allocation must align to in temporary allocation */
-constexpr int kTempAllocaAlignment = 64;
+constexpr int kTempAllocaAlignment = 128;


How about this? https://github.com/dmlc/tvm/blob/master/src/pass/ir_util.h#L174 I think we should handle it specially for Hexagon.

Someone suggested having a target-specific function that gets the required alignment, instead of having global constants. I think that would be a better solution. This patch simply has what we've done about it.
I haven't looked into it yet, is there a precedent for implementing such a thing? Do you have any suggestions as to how it should be done?

I also suggest we have a target-specific function that gets the required alignment. Then in GetTempAllocaAlignment we could get this alignment and just simply return it for Hexagon (Need detect the target is Hexagon, for example we define the macro USE_HEXAGON in CMakeLists.txt).

Same comment. How about create target-specific function alignment?

kparzysz-quic · 2019-05-13T19:46:00Z

The sim and device runtimes are now separated, and the header files for each are incorporated into the corresponding .cc files.

kparzysz-quic · 2019-05-15T14:27:14Z

Aside from the copyright issue, are there any other outstanding issues with the code? Have I missed anything?

kparzysz-quic · 2019-05-15T22:41:32Z

One common comment:
Currently, we only support CDSP. However, SnapDragon 820 is very popular in industry. 820 doesn't have CDSP and can not select. It only could go to ADSP. [...]
Could we consider supporting it?

We are considering support for ADSP.

Secondly, in fact, in HexagonSDK, we have UbuntuARM, and we could also support Linux and the code is also very similar. Could we have plan to support it?

I'll get back to you about this one. As of now we haven't done anything with UbuntuARM.

tqchen

some additional comments, overall look like we are converging

tqchen · 2019-05-16T00:29:35Z

src/runtime/hexagon/device/fastrpc/tvm_hexagon_remote.h

+ * permitted use with any of the "vta" and "verilog" subdirectories in the TVM
+ * repo.
+ */
+#ifndef TVM_RUNTIME_HEXAGON_DEVICE_FASTRPC_TVM_HEXAGON_REMOTE_H_


tvm_hexagon_remote.h ->hexagon_remote.h given TVM is implied

This file is automatically generated by the IDL compiler. It comes from a library called tvm_hexagon_remote, so its name becomes tvm_hexagon_remote.h. Even though it's edited after coming out of the IDL compiler (adding license, formatting, renaming the header guard), I'd like to keep the name to indicate the connection.

tqchen · 2019-05-16T00:33:04Z

src/runtime/hexagon/device/hexagon_cdsprpc_shim.cc

+
+#define WEAK __attribute__((weak))
+
+namespace {


consider use an explicit namespace anoymous namespace has problem udirng amalgmation

Document why this shim file is needed at the beginning of the file.

tqchen · 2019-05-16T00:37:16Z

src/runtime/hexagon/device/hexagon_cdsprpc_shim.cc

+
+namespace {
+
+void* LoadLibCdspRpc() {


consider make The CdspRPC a class(CDSPRPCSession, put all the fields into the class, and use Global singleton pattern). The redirection can get global singleton and run.

src/runtime/hexagon/hexagon_module.cc

src/runtime/hexagon/hexagon_module.h

src/runtime/hexagon/sim/driver/sim_device.cc

kparzysz-quic · 2019-05-22T18:11:35Z

As I mentioned in the last post. I think we should reduce the level of abstractions into two:

DeviceAPI, Module(tvm's original layer)

LowLevelDeviceAPI, (remove the intermediate hexagon::Device)

The reason is that we are essentially implementing a "driver" based on the low-level device interface. If the low-level interface is somewhat common between the simulator and real hardware, then we should only expose the LowLevelDeviceAPI.

I just looked at the uTVM PR, and the commonality is only superficial. Hexagon is in no way a micro-device in the sense of that PR.

tqchen · 2019-05-22T18:47:11Z

Sure, I just think there will be common design patterns that could be shared across the drivers. Given that you are already reducing the abstraction it is fine.

kparzysz-quic · 2019-06-25T12:30:31Z

This is an update of the original PR. Since there has been a month of a delay, I upgraded the sources to the latest ones (the code in the original PR got quite a bit out of date).
Hopefully all concerns are now addressed. The reason for the delay was a need for another legal review regarding license headers and our additional notices. The Apache license header is now identical to the one from Apache website.

tqchen · 2019-06-27T16:44:32Z

@FrozenGene @huajsj please help to do another round of review. I will also do a review in this week. Thanks @kparzysz-quic

FrozenGene · 2019-06-27T17:32:31Z

I will do the review next week. Thanks.

This Contribution is being provided by Qualcomm Innovation Center, Inc. See NOTICE file for more details.

kparzysz-quic · 2019-06-28T22:56:05Z

Rebased to resolve conflicts.

tqchen · 2019-06-30T23:55:56Z

I take a look at the new version of the license which contains the disclaimer text. We need to confirm again what is the implication especially after we migrate the repo to apache. I might wait until the repo migration before merging this.

While I feel it is OK, I would act caution and treat this differently(as if it was a different license). In particular, it would be great to have a clear isolated folder to point to, e.g. every file within src/runtime/hexagon has the disclaimer, while the rest of the files do not, and the disclaimer text can explicitly point to the subfolder, allowing the user to cut-off the content if necessary

NOTE: verilog folder has been removed and only vta folder will contain open source hardware design in the future.

kparzysz-quic · 2019-07-01T13:12:32Z

Is there a time frame for the repo migration?

FrozenGene · 2019-07-05T03:05:59Z

include/tvm/runtime/c_runtime_api.h

@@ -82,6 +82,7 @@ typedef enum {
  kDLSDAccel = 6,
  kOpenGL = 11,
  // AddExtraTVMType which is not in DLPack here


Move this comment after kDLHexagon = 13

FrozenGene · 2019-07-05T03:06:50Z

include/tvm/runtime/device_api.h


 /*! \brief Number of bytes each allocation must align to in temporary allocation */
-constexpr int kTempAllocaAlignment = 64;
+constexpr int kTempAllocaAlignment = 128;


Same comment. How about create target-specific function alignment?

FrozenGene · 2019-07-05T03:11:56Z

src/runtime/hexagon/device/hexagon_device_target.cc

+ * Qualcomm Technologies, Inc. and Qualcomm Innovation Center, Inc. retain
+ * copyright of their respective Contributions.
+ */
+#ifdef __ANDROID__


Could we try UbuntuARM64? From my experience, the code should be very similar with Android here? If we could add UbuntuARM64 support in this patch, I think it is better.

We don't have any experience with UbuntuARM64 or any devices that run it. We can consider it in the future, but it won't happen right away.

FrozenGene · 2019-07-05T03:14:52Z

src/runtime/hexagon/device/hexagon_stubapi.cc

+    TVM_LOGD("CDSP subsystem present");
+  } else if (!stat("/dev/subsys_adsp", &sb)) {
+    enable_domains_ = false;
+    TVM_LOGD("ADSP subsystem present");


Not review. Just clarify one information: So, ADSP is supported and work well, right?

Yes, ADSP is now supported on systems that have HVX.

FrozenGene · 2019-07-17T10:11:52Z

src/runtime/hexagon/device/hexagon_stubapi.cc

+    TVM_LOGD("ADSP subsystem present");
+  }
+
+  constexpr auto domain_lib_name = "libtvm_hexagon_remote_stub.so";


How do we generate this .so? Is it contained in the hexagon.cmake build process?

FrozenGene · 2019-07-17T10:12:00Z

@kparzysz-quic After reading https://discuss.tvm.ai/t/introducing-hexagon-backend/2421/13 again, about IDL preprocess, I think we could add one option like USE_HEXAGON_SDK. For example HEXAGON_IDL_PREPROCESS. when we set USE_HEXAGON is ON and HEXAGON_IDL_PREPROCESS is ON (default is false), we could run IDL compiler qaic. the IDL definition file could be existed in src/runtime/hexagon folder.

Currently, this patch contains the output of IDL, like tvm_hexagon_remote.h, right? But we could not find how to generate without IDL file.

FrozenGene · 2019-07-26T07:43:16Z

python/tvm/rpc/client.py

@@ -175,6 +175,10 @@ def ext_dev(self, dev_id=0):
        """Construct extension device."""
        return self.context(12, dev_id)

+    def hexagon(self, dev_id=0):
+        """Construct extension device."""


extension be Hexagon

FrozenGene · 2019-07-26T13:58:53Z

@kparzysz-quic I have used your PR to try to build runtime. I think we need tvm_hexagon_remote.idl and tvm_hexagon_remote_nd.idl, which could be located in fast_rpc directory , which could let users do it by themself or not. Meanwhile, we lack of stub.c / skel.c generated by IDL. We can not get the libtvm_hexagon_remote_stub.so and libtvm_hexagon_remote_nd_stub.so and so on.

Secondly, we need one procedure in build system or instruction to tell user to produce libtvm_hexagon_remote_skel.so / libtvm_hexagon_remote_stub.so. Like Halide does: https://github.com/halide/Halide/blob/master/src/runtime/hexagon_remote/Makefile

FrozenGene · 2019-09-10T12:01:43Z

src/runtime/hexagon/device/hexagon_device_target.cc

+}
+
+void HexagonTarget::CopyDeviceToDevice(void* dst, const void* src,
+                                       unsigned len) {}


When we execute model (such as NNVM frontend / Relay frontend), we will meet CopyDeviceToDevice, imagine we have module.get_output(0, tvm.nd.empty(output_shape, dtype="float32", ctx=ctx)).asnumpy(). The implementation is not difficult (Using GetAppsAddr to get src / dst app addr and memcpy). Leave this comment reminding us we should implement it and don't leave it empty. Otherwise, we will get 0 of output.

kparzysz-quic · 2020-04-06T17:27:38Z

Replaced by #5252.

tqchen changed the title ~~Initial implementation of Hexagon support~~ [RUNTIME] Initial implementation of Hexagon support May 9, 2019

tqchen added the status: need review label May 9, 2019

tqchen changed the title ~~[RUNTIME] Initial implementation of Hexagon support~~ [RUNTIME] Initial implementation of Hexagon runtime support May 9, 2019

tqchen requested changes May 10, 2019

View reviewed changes

tqchen self-assigned this May 10, 2019

FrozenGene reviewed May 10, 2019

View reviewed changes

junrushao reviewed May 10, 2019

View reviewed changes

tqchen reviewed May 11, 2019

View reviewed changes

src/runtime/hexagon/hexagon_module.cc Outdated Show resolved Hide resolved

FrozenGene reviewed May 13, 2019

View reviewed changes

tqchen requested changes May 16, 2019

View reviewed changes

tqchen requested changes May 17, 2019

View reviewed changes

src/runtime/hexagon/hexagon_module.h Outdated Show resolved Hide resolved

huajsj suggested changes May 19, 2019

View reviewed changes

src/runtime/hexagon/sim/driver/sim_device.cc Show resolved Hide resolved

[RUNTIME] Initial implementation of Hexagon runtime support

b4e5960

This Contribution is being provided by Qualcomm Innovation Center, Inc. See NOTICE file for more details.

FrozenGene reviewed Jul 5, 2019

View reviewed changes

FrozenGene reviewed Jul 17, 2019

View reviewed changes

FrozenGene reviewed Jul 26, 2019

View reviewed changes

FrozenGene reviewed Sep 10, 2019

View reviewed changes

kparzysz-quic closed this Apr 6, 2020

FrozenGene mentioned this pull request Apr 7, 2020

[RUNTIME] Initial implementation of Hexagon runtime support #5252

Merged

kparzysz-quic deleted the kparzysz-initial-hexagon-support branch April 10, 2020 20:34

tqchen mentioned this pull request Aug 24, 2022

[Runtime] Change default alignment to 64 bytes. #12564

Closed

		@@ -0,0 +1,366 @@
		/*!
		* Copyright 2018-2019 Qualcomm Innovation Center, Inc.

[RUNTIME] Initial implementation of Hexagon runtime support #3163

[RUNTIME] Initial implementation of Hexagon runtime support #3163

Conversation

kparzysz-quic commented May 9, 2019

junrushao commented May 9, 2019

kparzysz-quic commented May 9, 2019

kparzysz-quic commented May 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tqchen May 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tqchen commented May 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FrozenGene commented May 10, 2019 • edited Loading

junrushao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kparzysz-quic commented May 10, 2019

kparzysz-quic commented May 10, 2019

tqchen commented May 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kparzysz-quic commented May 13, 2019

kparzysz-quic commented May 15, 2019

kparzysz-quic commented May 15, 2019

tqchen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kparzysz-quic commented May 22, 2019 • edited Loading

tqchen commented May 22, 2019

kparzysz-quic commented Jun 25, 2019

tqchen commented Jun 27, 2019

FrozenGene commented Jun 27, 2019

kparzysz-quic commented Jun 28, 2019

tqchen commented Jun 30, 2019 • edited Loading

kparzysz-quic commented Jul 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FrozenGene commented Jul 17, 2019

Choose a reason for hiding this comment

FrozenGene commented Jul 26, 2019 • edited Loading

Choose a reason for hiding this comment

kparzysz-quic commented Apr 6, 2020

tqchen May 13, 2019 •

edited

Loading

FrozenGene commented May 10, 2019 •

edited

Loading

tqchen commented May 11, 2019 •

edited

Loading

kparzysz-quic commented May 22, 2019 •

edited

Loading

tqchen commented Jun 30, 2019 •

edited

Loading

FrozenGene commented Jul 26, 2019 •

edited

Loading