Skip to content

Conversation

@giuseros
Copy link
Contributor

@giuseros giuseros commented May 6, 2021

This PR aims at removing the function call to extract the parameters
within the AOT main function by introducing a tir::lookup_param builtin.

This has different benefits:

  • In AOT we now only use the v_handle field of the TVMValue struct
  • We save CPU cycles by not calling an intermediate function to extract
    local parameters
  • We reduce code size, since we don't need to pack a call to extract
    parameters and we don't need to produce the lookup_param function
    anymore within the compilation unit

Change-Id: I36c2f0724a79606424a4374f4f5cd669bb2a8a55

@giuseros
Copy link
Contributor Author

giuseros commented May 6, 2021

cc: @areusch @Mousius @manupa-arm

@tqchen tqchen changed the title AOT] Remove lookup parameter function in AOT [AOT] Remove lookup parameter function in AOT May 8, 2021
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

sorry for the delay @giuseros , just a couple small comments now.

@areusch areusch added the status: need update need update based on feedbacks label May 12, 2021
Giuseppe Rossini added 2 commits May 18, 2021 22:50
This PR aims at removing the function call to extract the parameters
within the AOT main function by introducing a tir::lookup_param builtin.

This has different benefits:
- In AOT we now only use the v_handle field
- We save cycles by not calling an intermediate function to extract
local parameters
- We reduce code size, since we don't need to pack a call to extract
parameters and we don't need to produce the lookup_param function
anymore within the compilation unit

Change-Id: I36c2f0724a79606424a4374f4f5cd669bb2a8a55
Change-Id: I83ba0189f559d310b5a80fe0bcc4d601b490d21a
@giuseros giuseros force-pushed the aot-remove-call-params branch from 3692704 to c7109ca Compare May 18, 2021 21:55
@giuseros
Copy link
Contributor Author

Hi @areusch sorry for the delayed reply. Somehow my github notifications were not working properly.

Change-Id: I84ab4a526d1284ded41fe95636e94c15412f6b28
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

thanks @giuseros !

@areusch areusch merged commit 71ff875 into apache:main May 20, 2021
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jun 17, 2021
* AOT] Remove lookup parameter function in AOT

This PR aims at removing the function call to extract the parameters
within the AOT main function by introducing a tir::lookup_param builtin.

This has different benefits:
- In AOT we now only use the v_handle field
- We save cycles by not calling an intermediate function to extract
local parameters
- We reduce code size, since we don't need to pack a call to extract
parameters and we don't need to produce the lookup_param function
anymore within the compilation unit

Change-Id: I36c2f0724a79606424a4374f4f5cd669bb2a8a55

* addressing comments

Change-Id: I83ba0189f559d310b5a80fe0bcc4d601b490d21a

* retrigger CI

Change-Id: I84ab4a526d1284ded41fe95636e94c15412f6b28
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jun 17, 2021
* AOT] Remove lookup parameter function in AOT

This PR aims at removing the function call to extract the parameters
within the AOT main function by introducing a tir::lookup_param builtin.

This has different benefits:
- In AOT we now only use the v_handle field
- We save cycles by not calling an intermediate function to extract
local parameters
- We reduce code size, since we don't need to pack a call to extract
parameters and we don't need to produce the lookup_param function
anymore within the compilation unit

Change-Id: I36c2f0724a79606424a4374f4f5cd669bb2a8a55

* addressing comments

Change-Id: I83ba0189f559d310b5a80fe0bcc4d601b490d21a

* retrigger CI

Change-Id: I84ab4a526d1284ded41fe95636e94c15412f6b28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: need update need update based on feedbacks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants