Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] [Hexagon] HVX conv2d kernel incorrectly relies on VTCM being zeroed #13144

Closed
supersat opened this issue Oct 19, 2022 · 2 comments
Closed
Labels
needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it type: bug

Comments

@supersat
Copy link
Contributor

supersat commented Oct 19, 2022

The HVX conv2d kernel introduced in #12204 only works in simulation, even with #13002 applied.

Zeroing out allocated VTCM memory in VTCMAllocation::VTCMAllocation() (a memset inserted at line 79 in hexagon_buffer.cc makes the kernel run correctly on real hardware. Conversely, filling a VTCM allocation with junk values (e.g., filling it with 0xCC) causes the kernel to fail in simulation as well.

It seems like something isn't being zeroed properly, or some VTCM is being accessed that shouldn't be. Unfortunately, I haven't been able to nail down where this happens yet.

Expected behavior

Executing the tests/python/contrib/test_hexagon/topi/test_conv2d_fp16_intrin.py tests should work on real hardware.

Actual behavior

Tests fail with result mismatches.

Environment

Ubuntu 20.04 with clang++ 14, TVM with PR #13002 applied, Hexagon SDK 5.1.0.0 with Hexagon Tools 8.5.13, using a development version of LLVM 16.

Steps to reproduce

  1. Set ANDROID_SERIAL_NUMBER to a valid Hexagon device serial number
  2. Run pytest tests/python/contrib/test_hexagon/topi/test_conv2d_fp16_intrin.py

Triage

  • needs-triage

cc: @kparzysz-quic @quic-sanirudh

cc @mehrdadh

@supersat supersat added needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it type: bug labels Oct 19, 2022
@quic-sanirudh
Copy link
Contributor

@supersat I actually have the fix ready already in my setup and was just verifying a couple of final things. I'll raise the PR soon, thanks for investigating.

@supersat
Copy link
Contributor Author

Resolved by #13165

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it type: bug
Projects
None yet
Development

No branches or pull requests

2 participants