You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/10/03 16:25:25 UTC
[GitHub] [tvm] areusch commented on a change in pull request #9103: [Core][Build] Move build module transformations and utilities to C++
areusch commented on a change in pull request #9103:
URL: https://github.com/apache/tvm/pull/9103#discussion_r720850402
##########
File path: python/tvm/driver/build_module.py
##########
@@ -297,28 +190,26 @@ def build(
m1 = tvm.lower(s1, [A, B, C], name="test_add1")
m2 = tvm.lower(s2, [A, B, C], name="test_add2")
rt_mod = tvm.build({"llvm": m1, "cuda": m2}, target_host="llvm")
-
Note
----
See the note on :any:`tvm.target` on target string format.
"""
- if isinstance(inputs, schedule.Schedule):
- if args is None:
- raise ValueError("args must be given for build from schedule")
+
+ if isinstance(inputs, (schedule.Schedule, tvm.IRModule, PrimFunc)):
+ # TODO(@mikepapadim) replace with te_lower
Review comment:
just wondering why the `if args is None` went away?
##########
File path: src/driver/driver_api.cc
##########
@@ -155,6 +156,13 @@ transform::Pass BindTarget(Target target) {
return tir::transform::CreatePrimFuncPass(fpass, 0, "BindTarget", {});
}
+transform::Pass AnnotateEntryFunc(bool b) {
Review comment:
should this be static as it's mainly helper, or do you envision others calling it from outside this module?
##########
File path: python/tvm/target/codegen.py
##########
@@ -36,7 +36,7 @@ def build_module(mod, target):
The corressponding module.
"""
target = Target(target) if isinstance(target, str) else target
- return _ffi_api.Build(mod, target)
+ return _ffi_api.Codegen(mod, target)
Review comment:
should we also rename the containing function, which is just a wrapper (which would really be renaming the contianing function to codegen but adding a backwards-compat `build_module` which warns about deprecation (there are some examples using warnings module i believe).
##########
File path: include/tvm/target/codegen.h
##########
@@ -45,7 +45,7 @@ using runtime::TVMRetValue;
* \param target The target to be built.
Review comment:
yeah it would in particular be great to identify the content of IRModule either by typedef or description in the comment.
##########
File path: src/driver/driver_api.cc
##########
@@ -373,88 +381,119 @@ TVM_REGISTER_GLOBAL("driver.lower_schedule")
return LowerSchedule(std::move(sch), args, name, c_binds, simple_mode);
});
-std::pair<IRModule, IRModule> SplitDevHostFuncs(IRModule mod_mixed, const Target& target_arg,
- const Target& target_host_arg,
- const transform::PassContext& pass_ctx) {
+// Splits module into one to run on the device and one to run the host. E.g., CUDA, OpenCL etc
+std::pair<IRModule, IRModule> SplitFuncsToDevHostMods(IRModule mod_mixed, const Target& target_arg,
Review comment:
this name is probably fine but I also don't mind SplitMixedModule, since it seems we are making "mixed module" a thing.
##########
File path: src/driver/driver_api.cc
##########
@@ -373,88 +381,119 @@ TVM_REGISTER_GLOBAL("driver.lower_schedule")
return LowerSchedule(std::move(sch), args, name, c_binds, simple_mode);
});
-std::pair<IRModule, IRModule> SplitDevHostFuncs(IRModule mod_mixed, const Target& target_arg,
- const Target& target_host_arg,
- const transform::PassContext& pass_ctx) {
+// Splits module into one to run on the device and one to run the host. E.g., CUDA, OpenCL etc
+std::pair<IRModule, IRModule> SplitFuncsToDevHostMods(IRModule mod_mixed, const Target& target_arg,
+ const Target& target_host_arg) {
Target target = target_arg, target_host = target_host_arg;
CheckAndUpdateHostConsistency(&target, &target_host);
- Array<tvm::transform::Pass> mixed_pass_list = {BindTarget(target),
- tir::transform::VerifyMemory()};
- mixed_pass_list.push_back(tir::transform::MergeDynamicSharedMemoryAllocations());
- if (pass_ctx->GetConfig<Bool>("tir.detect_global_barrier", Bool(false)).value()) {
- mixed_pass_list.push_back(tir::transform::ThreadSync("global"));
- }
- mixed_pass_list.push_back(tir::transform::ThreadSync("shared"));
- mixed_pass_list.push_back(tir::transform::ThreadSync("warp"));
- mixed_pass_list.push_back(tir::transform::InferFragment());
- mixed_pass_list.push_back(tir::transform::LowerThreadAllreduce());
+ ICHECK(mod_mixed.defined()) << "This module must be defined";
- if (target->GetAttr<Bool>("unpacked-api").value_or(Bool(false))) {
- mixed_pass_list.push_back(tir::transform::MakeUnpackedAPI());
- } else {
- mixed_pass_list.push_back(tir::transform::MakePackedAPI(-1));
- }
+ mod_mixed = OptimizeMixedModule(mod_mixed, target);
- mixed_pass_list.push_back(tir::transform::SplitHostDevice());
+ auto host_mod = OptimizeHostModule(mod_mixed, target_host);
- auto opt_mixed = transform::Sequential(mixed_pass_list);
- mod_mixed = opt_mixed(std::move(mod_mixed));
-
- auto host_pass_list = {
- Filter([](const tir::PrimFunc& f) {
- return f->GetAttr<Integer>(tvm::attr::kCallingConv, Integer(CallingConv::kDefault)) !=
- CallingConv::kDeviceKernelLaunch;
- }),
- BindTarget(target_host),
- tir::transform::LowerTVMBuiltin(),
- tir::transform::LowerCustomDatatypes(),
- tir::transform::LowerIntrin(),
- tir::transform::LowerDeviceStorageAccessInfo(),
- tir::transform::CombineContextCall(),
- };
- auto opt_host = transform::Sequential(host_pass_list);
- ICHECK(mod_mixed.defined()) << "This module must be defined";
- auto mhost = opt_host(mod_mixed);
-
- // device pipeline
- auto device_pass_list = {
- Filter([](const tir::PrimFunc& f) {
- return f->GetAttr<Integer>(tvm::attr::kCallingConv, Integer(CallingConv::kDefault)) ==
- CallingConv::kDeviceKernelLaunch;
- }),
- BindTarget(target),
- tir::transform::LowerWarpMemory(),
- tir::transform::Simplify(),
- tir::transform::LowerCustomDatatypes(),
- tir::transform::LowerIntrin(),
- tir::transform::LowerDeviceStorageAccessInfo(),
- };
- auto opt_device = transform::Sequential(device_pass_list);
- auto mdevice = opt_device(mod_mixed);
+ auto device_mod = OptimizeDeviceModule(mod_mixed, target);
// some final misc checks.
auto keys = target->GetKeys();
bool target_is_gpu = std::find(keys.begin(), keys.end(), "gpu") != keys.end();
- if (target_is_gpu && mdevice->functions.size() == 0) {
+ if (target_is_gpu && device_mod->functions.size() == 0) {
LOG(WARNING) << "Specified target " << target->str()
<< " but cannot find device code. Did you forget to bind?";
}
- if (target->kind->device_type == kDLCPU && target_host == target) {
- // TODO(@jroesch): This check is no longer true we need to figure out if we care about this.
- // We need to relax this check for just TIR functions.
- // ICHECK(mdevice->functions.empty()) << "No device code should be generated when target "
- // << "and host_target are both llvm target."
- // << "\n";
+ return {host_mod, device_mod};
+}
+
+std::pair<Target, Target> TargetTypeMangling(const Map<Target, IRModule>& inputs_arg, Target target,
+ Target target_host_arg) {
+ Target target_input_mod, target_host;
+
+ target = !target.defined() ? target.Current() : target;
Review comment:
this function seems pretty deep and there are the odd semantics with target being optionally supplied from a global. additionally, it's exported to frontends (Python). if the global export is the thing that makes this target override necessary, suggest splitting that behavior out so that it's easier to reason about the internal function.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org