Skip to content

[adreno] Fix avg_pool2d - #17

Open
lhez wants to merge 1 commit into
octoml:masterfrom
lhez:upstream/avgpool2d-fix
Open

[adreno] Fix avg_pool2d#17
lhez wants to merge 1 commit into
octoml:masterfrom
lhez:upstream/avgpool2d-fix

Conversation

@lhez

@lhez lhez commented May 2, 2022

Copy link
Copy Markdown
  • avg_pool2d is not properly vectorized, resulting in 4x duplicate work.
  • Because avg_pool2d has two stages, i.e. pool_sum and elemwise, the schedule function takes else branch, where vectorization on the innermost axis is missing.

* avg_pool2d is not properly vectorized, resulting in
  4x duplicate work (hence 4x worse performance).
* Because avg_pool2d has two stages, i.e. pool_sum and elemwise,
  the schedule function takes `else` branch, where vectorization
  on the innermost axis is missing. Whereas max_pool2d does not
  have pool_sum stage, hence it is properly vectorized in `if`
  branch.
@TejashShah
TejashShah requested a review from csullivan May 2, 2022 17:56
s[OL].vectorize(s[OL].op.axis[-1])
else:
s[Pool].compute_at(s[Out], tx)
s[Pool].vectorize(s[Pool].op.axis[-1])

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.

Thank you. LGTM, but I'd like to suggest small refactoring. After line 54 we could add OL = Pool. And after that we can remove this if condition and keep only true branch.

@elvin-n

elvin-n commented May 4, 2022

Copy link
Copy Markdown
Contributor

Are any performance numbers after this change? I suspect that vectorization of the pool stage happens in any case due to vectorization of the postops stage couple lines above s[Out].vectorize(s[Out].op.axis[-1])

@lhez

lhez commented May 4, 2022

Copy link
Copy Markdown
Author

Yes, we do see performance improvement by doing this vectorization (almost 4x). The perf gap compared to Mace was the main reason that we looked into this schedule.

@elvin-n

elvin-n commented May 5, 2022

Copy link
Copy Markdown
Contributor

just verified performance of of this change, it works great!

On SD888 I got inception v3 in fp16_accfp32 mode before this change (all intermediate tensors are buffers, pads are implemented through extending of tensor by addition kernel producing texture) - 42ms, with application of this change 40.6ms. Mace have 51.4ms, TensorflowLite 40.5ms.

@lhez

lhez commented May 9, 2022

Copy link
Copy Markdown
Author

@elvin-n Great! Really nice to know that you are able to reproduce the performance gain. By the way, I don't have much experience with TensorflowLite - does TensorflowLite use handwritten kernels like Mace, or is it some sort of compiler like TVM? It's a bit surprising that it outperform Mace so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants