Skip to content

Conversation

@Lunderberg
Copy link
Contributor

The primary goal of this PR is to add codegen for 2d physical layouts in CodeGenHexagon.

  • Correctly update buffer objects in IndexMap.
  • Extend CodeGenLLVM::BufferAccessHelper to allow for N-d buffer access. CodeGenLLVM is still limited to flat 1-d memory buffers, but this allows subclasses to support some forms of N-d buffers by overriding Subclass::CreateBufferPtr.
  • Implement CodeGenHexagon::CreateBufferPtr to allow for 2-d buffer access.
  • Added unit tests to define desired behavior. Currently, this only checks that the Hexagon module can be built. Tests to verify that the generated code will produce expected results depends on allocation support added in [Hexagon] Generalize builtin for Nd memory alloc with storage scope and add lowering for VTCM / Hexagon #10558.

Previous location in `CodeGenLLVM::BufferAccessHelper` occurred after
possible integer wrapping in `indices.size()-1` loop bounds.
Copy link
Contributor

@csullivan csullivan left a comment

Choose a reason for hiding this comment

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

LGTM!

@csullivan csullivan merged commit 8a636a9 into apache:main Mar 15, 2022
@csullivan
Copy link
Contributor

csullivan commented Mar 15, 2022

Thanks @Lunderberg @kparzysz-quic!

@Lunderberg Lunderberg deleted the hexagon_nd_load_store branch March 15, 2022 15:31
pfk-beta pushed a commit to pfk-beta/tvm that referenced this pull request Apr 11, 2022
* Added unit tests for codegen of 2d physical buffers in Hexagon.

* Update IndexMap when buffers are updated.

* Extended CodeGenLLVM::BufferAccessHelper to support N-d

This way, a subclass can override GetBufferPtr, without needing to
reimplement all of the other indexing logic for
BufferLoad/BufferStore.

* Updated CodeGenHexagon to treat 2-d physical buffers as T**

* Moved indices size check earlier.

Previous location in `CodeGenLLVM::BufferAccessHelper` occurred after
possible integer wrapping in `indices.size()-1` loop bounds.

* Updated to use `llvm::ArrayRef` instead of `std::vector`.

* Resolve lint error.

* CI fix, contextlib.nullcontext not available on python3.6
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