[Runtime] Enable option to use OpenMP thread pool #4089

icemelon · 2019-10-08T17:48:43Z

I found that when TVM uses MKL library, two different threadpool implementation competes for CPUs and leads to pool performance. This PR adds an option in the compiler flag to use OpenMP threadpool. It also helps TVM integration with MxNet as it uses OpenMP threadpool.

cc @yidawang @yzhliu @tqchen

tmoreau89

Can we also test OpenMP in the CI? We may have to update the Dockerfile etc. so that can be done in a subsequent PR.

yidawang

In addition, the existing thread pool can distinguish big/small cores of ARM, decide whether or not to use the master thread to run task 0, etc. If adding an omp option, we should match the functionality of them as well.

src/runtime/thread_pool.cc

yidawang · 2019-10-08T19:18:26Z

src/runtime/thread_pool.cc

+    std::atomic<int32_t>* sync_counter = new std::atomic<int>[num_task * tvm::runtime::kSyncStride];
+    for (int i = 0; i < num_task; ++i) {
+      sync_counter[i * tvm::runtime::kSyncStride].store(
+          0, std::memory_order_relaxed);
+    }
+    env.sync_handle = sync_counter;


This part assumes that need_sync==1, which is the de facto default now, but we probably need to keep the same logical behavior with the existing thread pool. Another option is to remove need_sync from the code. @tqchen Do you want to talk about if we still want need_sync in ThreadPool::Launch()? https://github.com/dmlc/tvm/blob/master/src/runtime/thread_pool.cc#L298

need_sync is used to generate barriers. if we do openmp, perhaps we can ignore it and just use omp's barrier

junrushao · 2019-10-13T23:15:24Z

CMakeLists.txt

+if(USE_OMP)
+  message(STATUS "Build with OpenMP")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fopenmp")
+  add_definitions(-DUSE_OMP=1)


Would it be slightly better to use target_compile_definitions?

tqchen · 2019-10-14T04:01:58Z

src/runtime/thread_pool.cc

@@ -394,12 +397,34 @@ int TVMBackendParallelLaunch(
    FTVMParallelLambda flambda,
    void* cdata,
    int num_task) {
+#ifndef USE_OMP


consider change the macro to be TVM_THREADPOOL_USE_OMP to be more project specific. Add a default value likie other cases.

soiferj · 2019-10-11T17:06:42Z

CMakeLists.txt

@@ -154,6 +155,12 @@ list(APPEND COMPILER_SRCS ${RELAY_BACKEND_SRCS})
 list(APPEND COMPILER_SRCS ${RELAY_IR_SRCS})
 list(APPEND COMPILER_SRCS ${RELAY_QNN_SRCS})

+if(USE_OMP)
+  message(STATUS "Build with OpenMP")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fopenmp")


Can you please make sure this works on Windows? From a quick search, it looks like you should use /openmp.

Jon, I updated the cmakefile. It'd work for Windows. However, I don't have a Windows dev machine. Could you help me verify whether it works on Windows?

Checking now.

On Windows, the file is libiomp5md.lib, from the docs here.

Can you please update the Intel block to look like this? It works for me.

if(MSVC) find_library(OMP_LIBRARY NAMES libiomp5md) else() find_library(OMP_LIBRARY NAMES iomp5) endif()

Also, this relies on the OMP library being in the PATH, correct?

Thanks, Jon!
Yes, we can potentially add an option for openmp path just like MKL. Do you it's necessary?

It's probably not necessary for now, we can always add it in a later change :) also, just to make sure, when using OpenMP, TVM_NUM_THREADS will not be used, right? We will only use OMP_NUM_THREADS?

You can use either TVM_NUM_THREADS or OMP_NUM_THREADS, but OMP_NUM_THREADS is recommended since it is effective to 3rd party library using OMP as well.

You can see the number of threads is determined at
https://github.com/dmlc/tvm/pull/4089/files#diff-f67700f4f293bfeb6dbc32bd659f4e48R405
https://github.com/dmlc/tvm/blob/master/src/runtime/threading_backend.cc#L218

src/runtime/thread_pool.cc

tqchen · 2019-10-17T20:48:19Z

ping @icemelon9

cmake/modules/OpenMP.cmake

icemelon · 2019-10-18T23:10:49Z

@tqchen @yidawang Could you take a look again?

tqchen · 2019-10-20T17:40:28Z

Thanks @icemelon9 @soiferj @yidawang @junrushao1994 @tmoreau89 !

* [relay][vm] Separate VM runtime with executable (apache#4100) * [relay][vm] Separate VM runtime with executable * Address comments * move ctx back to vm * make only vm related fields and methods protected * integrate seriliaztion/deserialization to executable * create stream * [Relay][Frontend][TF] Add tensor array ops (apache#3798) * [Relay][Frontend][TF] Add tensor array ops * rename * delete test * Move utility function * Refactor * fix tensor array ops * fix test * fix rebase * Fix serializer bug * Improve tf convert name lookup to use prelude api * Fix lint * Fix test * Fix typo (apache#4144) * [CI] Pin NNPack pthreadtools version (apache#4152) * [QNN][TFLite] Parsing QNN Add op. Adding MobilenetV2. (apache#4142) * Add lift_if_then_else pass (apache#3865) * Add LiftIfThenElse pass * Add more comments * Rename and refactor * Add description for internal data structure * Rename a test * Minor change * Address comments * Improve update_for * [CI] Update cpu docker (apache#4153) * [Refactor] Rename Datatype to ADT (apache#4156) We think it will reduce the confusion with the meaning. https://discuss.tvm.ai/t/discuss-consider-rename-vm-datatype/4339 * [Runtime] Enable option to use OpenMP thread pool (apache#4089) * [REFACTOR][NODE][RUNTIME] Move Node to the new Object protocol. (apache#4161) * [REFACTOR][NODE][RUNTIME] Move Node to the new Object protocol. This PR removes the original node system, and make node as a subclass of Object. This is a major refactor towards a better unified runtime object system. List of changes in the refactor: - We now hide data_ field, use Downcast explicitly to get a sub-class object. - Removed the node system FFI in python. - Removed the node C API, instead use PackedFunc for list and get attrs. - Change relay::Op::set_attr_type_key(attr_key_name) to relay::Op::set_attr_type<AttrType>(). - This change was necessary because of the new Object registration mechanism. - Subsequent changes to the op registrations - The change revealed a few previous problems that is now fixed. - Patched up a few missing node type registration. - Now we will raise an error if we register object that is not registered. - The original node.h and container.h are kept in the same location. - Calling convention: kObjectHandle now equals the old kNodeHandle, kNodeHandle is removed. - IRFunctor now dispatches on ObjectRef. - Update to the new type checking API: is_type, derived_from are replaced by IsInstance. - Removed .hash member function, instead use C++ convention hasher functors. * Address review comments * [CI] Move golang tests to the end (apache#4164) * Add support for quantized multiply to Relay (apache#4141) This patch adds multiply operator for quantized tensors. The details of the quantized multiplication are outlined in the code. This builds on pull request 3927 and includes the changes Animesh mentions in the comments on that request. Change-Id: I555715b53d0266a91d5c03dc3dfe8fc31e7ce4e1 * Fix missspelling (apache#4166) FIX "After connecting he usb" with "After connecting the usb" * [Relay][Pass] Count MAC for BatchMatMul (apache#4157) * count MAC for BatchMatMul * update doc * [Relay][QNN] Add unit test for int8 (apache#4159) * [bugfix][codegen] fix casting bug in llvm codegen * update example * retrigger ci * check llvm version * [relay][vm] Reuse allocated device memory (apache#4170) * add missing gradient check to gradient pass (apache#4169) * merge extract_from_program and extract_from_multiple_progam (apache#4173) * [TOPI] Added support for Mali Bifrost target (apache#4047) * [Relay][Frontend][TF] Fix Size operator (apache#4175) * [Relay][Frontend][TF] Fix Size operator * Uncomment tests * [Pass] Remove dead code (apache#4177) * [rpc] use callback func to do send & recv (apache#4147) * [rpc] use callback func to do send & recv. don't get fd from sock as it is deprecated in java * fix java build * fix min/max macro define in windows * keep the old rpc setup for py * add doc for CallbackChannel * Add support and testing for tf.assert (as no-op) and tf.no_op to TF Relay frontend. (apache#4172) * [DOCS] Add TensorFlow frontend docs (apache#4154) * Start to update TF frontend docs * Add rst * Remove markdown * Update wording * Resolve comments * Revert "[Relay][QNN] Add unit test for int8 (apache#4159)" (apache#4192) This reverts commit 6f9d028. * [cmake][ANTLR] Support setting path to ANTLR jar (apache#4176) * Support setting path to ANTLR jar * Update comment * Split adaptive_pool2d_avg into sum and div (apache#4186) * [Documentation]Fix example code in comment of tvm.build_module.build() (apache#4195) * Fix example code in comment of tvm.build_module.build() * Update build_module.py * [relay] use time_evaluator for measurement (apache#4191) * Add parser support for SUM tflite operator (apache#4182) * [Relay] Fix memory leak in the interpreter (apache#4155) * save lint * address reviewer comment * [TOPI] Tunable Template for Conv2D HWCN on CUDA (apache#4168) * support conv2d HWCN in AutoTVM and Relay * fix lint * fix comments and unit tests * TensorCore Support using Intrinsic (apache#4136) * add tensor core support * avoid memory bank conflict * fix thread sync & better performance * better performance * add schedule test for conv2d * extend into BatchMatMul * support config fragment shape and layout using intrinsic * add TensorCore tutorial * add int support and fix lint * address comment * add 32*16*8 TensorCore test * fix wmma include logic * [NODE][REFACTOR] Refactor reflection system in node. (apache#4189) * [NODE][REFACTOR] Refactor reflection system in node. - Removed the old Node, Node is now just an alias of runtime::Object - Introduce ReflectionVTable, a new columnar dispatcher to support reflection - This allows us to remove vtable from most node objects - The VisitAttrs are registered via TVM_RESGITER_NODE_TYPE, they are no longer virtual. - Consolidated serialization and reflection features into node. * Explicit type qualification when calling destructor. * Fix SPIRV, more comments * hotfix the ci (apache#4199) * [TOPI][x86] Legalize - Support int8xint8 convolution to use VNNI instructions. (apache#4196) * [Relay] crossentropy_with_logits and its gradient (apache#4075) * save * lint * [hotfix] missing include headers (apache#4204) * [Relay][Training] Add checkpoint annotation for checkpointing memory optimization (apache#4146) * add checkpoint annotation for checkpointing memory optimization * add alpha-equivalence checkpoint test and fix gradient type issue * fix build issues * ignore checkpoint annotation when checking missing gradients * refactor, fix checkpoint compute for tuple and add tests * [Relay][Params] Add APIs for storing and retrieving parameters from individual functions. (apache#4194) * Add support for attaching params * Fix types * Fix test * [Relay][Frontend][ONNX] Add support for op Where (apache#4184) * Add support for op Where * Update impl version * [VTA][Chisel] TSIM VTA Source Refactor (apache#4163) * app init push * fix on readme * change name, add bit serial explanantion * rm serialLoadMM, change doc * syntax change for readme * add parallel test functionality * fix readme * add python doc * syntax * init commit * fix empty line * fix typo * [RUNTIME] Separate runtime related contrib into runtime/contrib (apache#4207) * Fix type var docs (apache#4208) * [Relay] Setting Legalize opt_level to 1. (apache#4198) * [TOPI] Fix flaky testcase for check round (apache#4211) * [Relay][Op] Enhance Upsample Operator to support float scales (apache#4206) * :add scale2 for upsample * update unit test for upsampling * support latest upsample op for multiple frontend * fix lint * fix lint * fix lint * fix lint * update scale description and rebase * [Relay][Quantize] Use fixed point mulplications (apache#4160) * Update have_int8 condition to run on compute capability 7.x devices (apache#4214) * Optimizing autotvm task extraction speed (apache#4138) * Optimize task extraction speed * correct pylint errors * Delete unused function * remove unnecessary argument * resolve code review comments * corrent cpp lint errors * remove one more graph_json return value * fix test bugs * [Relay] Add Python type functor and tests (apache#4209) * Add Python type functor and tests * Lint roller * Fix typo in packed_func.h (apache#4219) * Improve the lowering of Qnn Dense (apache#4213) * [QNN] Improving Dense lowering. * - Moving get_shape method to util - Finalizing the test cases and the code structure for optimized dense computation. * - Fixing cpplint. * - Addressing review comments. * - Renaming the variables correctly. * - Renaming the variables correctly. * [ARITH] Fix the rule y < x && x <= y (apache#4220) * [PYTHON] Add __init__ to the generated grammar so that it can be installed properly (apache#4223) * [Relay][Frontend][ONNX] New Operators and Opsets to Support BERT (apache#4197) * Added slice v10 * Added constantofshape operation and small refactor. * Finished one_hot implementation. * Reshape working across all bert layers. * Fixed constantofshape and removed code duplication. * onnx model fully ingested. * Working on improving onnx tests. * Changed onnx testing to use onnxruntime instead of caffe2, also formatted. * Add arbitrary output nodes to onnx frontend. * Added v6 tiling for bert squad 8 support. * Small syntax fixes * Reduced code duplication in split opset versions. * Added batch matmul test * Added unstack split testing. * Adde onehot test, needs a little cleanup probably. * Replaced deprecated constant fill with constantofshape and updated tests accordingly. * Added tests for new opset version of slice and tile. * lint clean up * Lint fixes * Changed onnx dependency * Went back to caffe2 runtime for CI integration. * Rebase and small typo/syntax changes. * Added hard casting of onehot attributes to int. * [Relay][Topi][TensorFlow][ONNX][Lang] Add support for Any op (apache#4205) * Add support for Any op * Support ONNX frontend * Add doc * Add to relay docs * Dummy change to retrigger CI * Update dmlc_tvm_commit_id.txt * Merge from upstream

[Runtime] Enable option to use OpenMP thread pool (apache#4089)

icemelon added 3 commits October 8, 2019 17:41

[TVM] Add an option to use OMP thread pool

8c96d8b

fix

36782fb

lint

995682e

tmoreau89 reviewed Oct 8, 2019

View reviewed changes

yidawang suggested changes Oct 8, 2019

View reviewed changes

tqchen added the status: need update need update based on feedbacks label Oct 10, 2019

fix

d53403e

junrushao reviewed Oct 13, 2019

View reviewed changes

tqchen requested changes Oct 14, 2019

View reviewed changes

soiferj suggested changes Oct 15, 2019

View reviewed changes

icemelon added 3 commits October 18, 2019 21:02

update

880f4a1

fix

097660f

clean up

984cd50

soiferj reviewed Oct 18, 2019

View reviewed changes

cmake/modules/OpenMP.cmake Outdated Show resolved Hide resolved

icemelon added 2 commits October 18, 2019 21:21

fix

794da1a

fix for windows

dd340e3

soiferj reviewed Oct 18, 2019

View reviewed changes

cmake/modules/OpenMP.cmake Outdated Show resolved Hide resolved

fix

115cac5

soiferj approved these changes Oct 18, 2019

View reviewed changes

yidawang approved these changes Oct 19, 2019

View reviewed changes

junrushao approved these changes Oct 19, 2019

View reviewed changes

tqchen approved these changes Oct 20, 2019

View reviewed changes

tqchen merged commit 97ea31c into apache:master Oct 20, 2019

tqchen added status: accepted and removed status: need update need update based on feedbacks labels Oct 20, 2019

icemelon deleted the omp branch October 21, 2019 16:02

kevinthesun pushed a commit to kevinthesun/tvm that referenced this pull request Oct 30, 2019

[Runtime] Enable option to use OpenMP thread pool (apache#4089)

ffc11b7

tqchen mentioned this pull request Nov 8, 2019

[RELEASE][DRAFT] TVM v0.6 Release candidate #4259

Closed

reminisce mentioned this pull request Nov 11, 2019

Update TVM submodule apache/mxnet#16777

Merged

yangchen-MS pushed a commit to microsoft/onnxruntime-tvm that referenced this pull request Nov 27, 2019

[Runtime] Enable option to use OpenMP thread pool (apache#4089)

740f61f

ke1337 pushed a commit to microsoft/onnxruntime-tvm that referenced this pull request Nov 27, 2019

Merge pull request #8 from microsoft/yanchen/nuphar/openmp

c6e3efc

[Runtime] Enable option to use OpenMP thread pool (apache#4089)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Runtime] Enable option to use OpenMP thread pool #4089

[Runtime] Enable option to use OpenMP thread pool #4089

icemelon commented Oct 8, 2019

tmoreau89 left a comment

yidawang left a comment

yidawang Oct 8, 2019

tqchen Oct 10, 2019

icemelon Oct 11, 2019

junrushao Oct 13, 2019

tqchen Oct 14, 2019

icemelon Oct 18, 2019

soiferj Oct 11, 2019

icemelon Oct 18, 2019

soiferj Oct 18, 2019

soiferj Oct 18, 2019 •

edited

Loading

icemelon Oct 18, 2019

soiferj Oct 18, 2019

icemelon Oct 18, 2019 •

edited

Loading

tqchen commented Oct 17, 2019

icemelon commented Oct 18, 2019 •

edited

Loading

tqchen commented Oct 20, 2019

[Runtime] Enable option to use OpenMP thread pool #4089

[Runtime] Enable option to use OpenMP thread pool #4089

Conversation

icemelon commented Oct 8, 2019

tmoreau89 left a comment

Choose a reason for hiding this comment

yidawang 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

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

soiferj Oct 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

icemelon Oct 18, 2019 • edited Loading

Choose a reason for hiding this comment

tqchen commented Oct 17, 2019

icemelon commented Oct 18, 2019 • edited Loading

tqchen commented Oct 20, 2019

soiferj Oct 18, 2019 •

edited

Loading

icemelon Oct 18, 2019 •

edited

Loading

icemelon commented Oct 18, 2019 •

edited

Loading