Skip to content

Pass resource_handle to operators with unpacked API - #8452

Closed
Mousius wants to merge 2 commits into
apache:mainfrom
Mousius:resource-handle-unpacked
Closed

Pass resource_handle to operators with unpacked API#8452
Mousius wants to merge 2 commits into
apache:mainfrom
Mousius:resource-handle-unpacked

Conversation

@Mousius

Copy link
Copy Markdown
Member

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:

int32_t__tvm_main__(void*args, void*type_code, intnum_args, void*out_value, void*out_type_code, void*resource_handle) {
returntvmgen_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_DLLint32_ttvmgen_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);
return0;
}

@areusch@manupa-arm

@Mousius
Mousiusforce-pushed the resource-handle-unpacked branch 2 times, most recently from 4e8e6e3 to 003135aCompareJuly 13, 2021 16:15
@Mousius
Mousiusforce-pushed the resource-handle-unpacked branch from 003135a to d488448CompareJuly 22, 2021 14:42
@Mousius
Mousiusforce-pushed the resource-handle-unpacked branch 4 times, most recently from 782928e to 203a51fCompareJuly 29, 2021 19:23
*/
Map<tir::Var, Buffer> buffer_map;
/*! \brief The resource handle to be used by the function when accessing platform resources */
tir::Var resource_handle;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like this should be a property of the call site rather than a function. how come you want to add it here?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Mousiusforce-pushed the resource-handle-unpacked branch from 203a51f to c69c994CompareAugust 4, 2021 12:43
@junrushao

Copy link
Copy Markdown
Member

This PR changes the signature of PrimFunc, but I am not sure I am the best guy to decide if it is the best way to do this. CC: @tqchen@jroesch

@areuschareusch added the status: need update need update based on feedbacks label Aug 10, 2021
@junrushaojunrushao added the status: need RFC need RFC discussion label Aug 22, 2021
@Mousius

Copy link
Copy Markdown
MemberAuthor

@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 😸

@areusch

Copy link
Copy Markdown
Contributor

@Mousius Let's just resolve the places in which we want to pass through resource_handle on the pre-RFC thread first.

@Mousius

Mousius commented Nov 16, 2021

Copy link
Copy Markdown
MemberAuthor

Replaced by #9395 / #9501

@MousiusMousius closed this Nov 16, 2021
@Mousius
Mousius deleted the resource-handle-unpacked branch November 16, 2021 09:43
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: need RFCneed RFC discussionstatus: need updateneed update based on feedbacks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Mousius@junrushao@areusch