[Relay] Introduce Executor and Runtime representations with associated registries #9246
Conversation
Mousius
requested review from
anijain2305,
areusch,
comaniac,
jroesch,
junrushao,
jwfromm,
MarisaKirisame,
mbrookhart,
merrymercy,
slyubomirsky,
tqchen,
vinx13,
wweic,
yzhliu,
zhiics and
ZihengJiang
as code owners
Mousius
force-pushed
the
runtime-executor-infra
branch
2 times, most recently
from
84420ae
to
94c7e34
Compare
| * | ||
| * \sa Executor | ||
| */ | ||
| class ExecutorNode : public Object { |
There was a problem hiding this comment.
Intended to be sub-classable right? Ie I don't have to reflect everything I care about into the generic attrs?
There was a problem hiding this comment.
At the moment all the options are enclosed in the attributes, but I left it open for extension if we want to do class AOTExecutorNode : ExecutorNode or similar - I see this being more of a thing with Runtime which may well represent the runtime itself at some point.
| * \brief Helper structure to register Executors | ||
| * \sa TVM_REGISTER_EXECUTOR | ||
| */ | ||
| class ExecutorRegEntry { |
There was a problem hiding this comment.
Any hope of templating away all the gobs of boilerplate? Forgive me if there's an obvious catch I can't see.
There was a problem hiding this comment.
I started down this path but realised that if we wanted to do this it'd make sense to do it in the context of:
TargetKind(ValueTypeInfo / RegEntry)PassConfigManager(ValueTypeInfo)Op(OpRegEntry)InstructionKind(InstructionKindRegEntry)
I started doing this and ended up with what was essentially a templated mess - so I'd rather take it later and in isolation 😸
|
|
||
| /********** Register Runtimes and options **********/ | ||
|
|
||
| TVM_REGISTER_RUNTIME("c").add_attr_option<Bool>("system-lib"); |
There was a problem hiding this comment.
before we were using crt which i admit is a bit of a double-naming. however, we also use crt to identify the C runtime in the source tree...so i sort of also see it as a name in and of itself.
additionally, we have quite a few more attributes that could be moved. just curious what your plan is here?
There was a problem hiding this comment.
I can rename to crt in the next patch in the series, so that's simples.
Which other attributes were you meaning? I think I missed workspace-byte-alignment for AOT but I can't really dissect the other attributes on the other Targets, would appreciate some help and happy to tweak this list further as I get to the next patch 😸
|
|
||
| TVM_REGISTER_EXECUTOR("graph").add_attr_option<Bool>("link-params", Bool(false)); | ||
|
|
||
| TVM_REGISTER_EXECUTOR("vm"); |
There was a problem hiding this comment.
should we register this now @mbs-octoml or after we actually merge all the compilation flows?
There was a problem hiding this comment.
Whatever the outcome, I can add/remove it in the next PR in the series 😸
There was a problem hiding this comment.
I'm not fussed -- the code paths are so independent at the moment the real work is to come anyway.
|
|
||
| /********** Register Executors and options **********/ | ||
|
|
||
| TVM_REGISTER_EXECUTOR("aot") |
There was a problem hiding this comment.
although we don't use the original link-params method at runtime for AOT, don't we use it as an enable for certain things in the AOT compilation flow?
|
@areusch, just to clarify, this lands the infrastructure to start generating arguments in line with https://github.com/apache/tvm-rfcs/blob/main/rfcs/0028-command-line-registry-composition.md and work my way through the options there - so I expect a bit of change in the actual attrs when they're wired end-to-end (including |
…d registries This is the underpinning Executor/Runtime objects for apache/tvm-rfcs#29, this doesn't change the wiring as yet since that's a pretty big change in itself. Most of this patch is TVM boilerplate, which I've tried to minimize - there's maybe some future work to decide how to more easily define some of these things.
Mousius
force-pushed
the
runtime-executor-infra
branch
from
94c7e34
to
a81af63
Compare
…d registries (apache#9246) This is the underpinning Executor/Runtime objects for apache/tvm-rfcs#29, this doesn't change the wiring as yet since that's a pretty big change in itself. Most of this patch is TVM boilerplate, which I've tried to minimize - there's maybe some future work to decide how to more easily define some of these things.
…d registries (apache#9246) This is the underpinning Executor/Runtime objects for apache/tvm-rfcs#29, this doesn't change the wiring as yet since that's a pretty big change in itself. Most of this patch is TVM boilerplate, which I've tried to minimize - there's maybe some future work to decide how to more easily define some of these things.
This is the underpinning Executor/Runtime objects for apache/tvm-rfcs#29, this doesn't change the wiring as yet since that's a pretty big change in itself. Most of this patch is TVM boilerplate, which I've tried to minimize - there's maybe some future work to decide how to more easily define some of these things.