Pass resource_handle to operators with unpacked API #8452
Conversation
Mousius
force-pushed
the
resource-handle-unpacked
branch
2 times, most recently
from
4e8e6e3 to
003135a
Compare
Mousius
force-pushed
the
resource-handle-unpacked
branch
from
003135a to
d488448
Compare
Mousius
requested review from
MarisaKirisame,
ZihengJiang,
anijain2305,
areusch,
comaniac,
jroesch,
junrushao,
jwfromm,
kparzysz-quic,
masahi,
mbrookhart,
merrymercy,
slyubomirsky,
tqchen,
vinx13,
wweic,
yzhliu and
zhiics
as code owners
Mousius
force-pushed
the
resource-handle-unpacked
branch
4 times, most recently
from
782928e to
203a51f
Compare
| */ | ||
| Map<tir::Var, Buffer> buffer_map; | ||
| /*! \brief The resource handle to be used by the function when accessing platform resources */ | ||
| tir::Var resource_handle; |
There was a problem hiding this comment.
it seems like this should be a property of the call site rather than a function. how come you want to add it here?
There was a problem hiding this comment.
This is referring to the variable used in the function to pass into further calls, similar to the params above except we don't treat it as a parameter which would get packed.
There was a problem hiding this comment.
hmm, but is there just one resource_handle associated with each function? suppose you had two instances of the same accelerator and wanted to launch the same compute function twice concurrently? also, what about the AOT top-level function, which should have a struct like TVMDevices_my_model?
This patch passes the resource_handle variable through the AOT executor
down to the backend operators. In order to model this on the `PrimFunc`
the resource_handle was added as a property, the resource_handle property
is then added to the arguments when transformed via MakeUnpackedAPI.
The flow of the resource_handle looks similar to this:
```c
int32_t __tvm_main__(void* args, void* type_code, int num_args, void* out_value, void* out_type_code, void* resource_handle) {
return tvmgen_run_model(
((DLTensor*)(((TVMValue*)args)[0].v_handle))[0].data,
((DLTensor*)(((TVMValue*)args)[1].v_handle))[0].data,
((DLTensor*)(((TVMValue*)args)[2].v_handle))[0].data,
((DLTensor*)(((TVMValue*)args)[3].v_handle))[0].data,
resource_handle
);
}
TVM_DLL int32_t tvmgen_run_model(void* arg0, void* arg1, void* arg2, void* arg3, void* resource_handle) {
void* input = arg0;
void* input1 = arg1;
void* input2 = arg2;
void* output = arg3;
(void)tvmgen_fused_concatenate_add(input, input1, input2, output, resource_handle);
return 0;
}
```
Black autoformats this to a longer line than pylint allows, only marking the relevant variable causes the formatting to run correctly. I think this is fine based on the assumption that the ignores should be removed in favour of proper typing at some point.
Mousius
force-pushed
the
resource-handle-unpacked
branch
from
203a51f to
c69c994
Compare
|
@junrushao1994 / @areusch I've posted https://discuss.tvm.apache.org/t/pre-rfc-c-device-api/10874 to motivate the changes here, but given this is a simple wiring exercise if the assumptions sound reasonable to you it'd be good to review this now 😸 |
|
@Mousius Let's just resolve the places in which we want to pass through |
3 participants
This patch passes the resource_handle variable through the AOT executor
down to the backend operators. In order to model this on the
PrimFuncthe resource_handle was added as a property, the resource_handle property
is then added to the arguments when transformed via MakeUnpackedAPI.
The flow of the resource_handle looks similar to this:
@areusch @manupa-arm