Skip to content

Conversation

@Lunderberg
Copy link
Contributor

@Lunderberg Lunderberg commented Jul 6, 2023

Prior to this commit, the call to TVMBackendFreeWorkspace occurred outside the LetStmt that defined the workspace pointer. While works with current codegen, as the code produced for LetStmt does not check for out-of-scope access, this access of an out-of-scope should be avoided.

In addition, the AttrStmt providing the storage alignment was placed outside the LetStmt that defines the variable. As a result, the alignment assumption is never actually used, as CodeGenLLVM::VisitStmt_(const AttrStmtNode*) only creates an alignment assumption for in-scope variables.

This PR updates LowerTVMBuiltin to produce both the storage alignment AttrStmt and the call to TVMBackendFreeWorkspace inside the body of LetStmt, rather than outside.

Prior to this commit, the call to `TVMBackendFreeWorkspace` occurred
outside the `LetStmt` that defined the workspace pointer.  While works
with current codegen, as the code produced for `LetStmt` does not
check for out-of-scope access, this access of an out-of-scope should
be avoided.

This commit updates `LowerTVMBuiltin` to produce the call to
`TVMBackendFreeWorkspace` at the end of the `LetStmt`'s body, rather
than just after the `LetStmt`.
@tvm-bot
Copy link
Collaborator

tvm-bot commented Jul 6, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

Prior to this commit, the `AttrStmt` providing the storage alignment
was placed outside the `LetStmt` that defines the variable. As a
result, the alignment assumption is never actually used, as
`CodeGenLLVM::VisitStmt_(const AttrStmtNode*)` only creates an
alignment assumption for in-scope variables.

This commit moves the storage alignment `AttrStmt` to be inside the
`LetStmt`, rather than outside.
@Lunderberg Lunderberg force-pushed the lower_tvm_builtin_free_inside_letstmt branch from 00749e6 to 1a0fe67 Compare July 6, 2023 16:47
@Hzfengsy Hzfengsy merged commit 30d6842 into apache:main Jul 10, 2023
@Lunderberg Lunderberg deleted the lower_tvm_builtin_free_inside_letstmt branch July 10, 2023 13:07
junrushao pushed a commit to junrushao/tvm that referenced this pull request Jul 24, 2023
* [TIR] Call TVMBackendFreeWorkspace inside LetStmt

Prior to this commit, the call to `TVMBackendFreeWorkspace` occurred
outside the `LetStmt` that defined the workspace pointer.  While works
with current codegen, as the code produced for `LetStmt` does not
check for out-of-scope access, this access of an out-of-scope should
be avoided.

This commit updates `LowerTVMBuiltin` to produce the call to
`TVMBackendFreeWorkspace` at the end of the `LetStmt`'s body, rather
than just after the `LetStmt`.

* [TIR] Output AttrStmt "storage_alignment" inside the var binding

Prior to this commit, the `AttrStmt` providing the storage alignment
was placed outside the `LetStmt` that defines the variable. As a
result, the alignment assumption is never actually used, as
`CodeGenLLVM::VisitStmt_(const AttrStmtNode*)` only creates an
alignment assumption for in-scope variables.

This commit moves the storage alignment `AttrStmt` to be inside the
`LetStmt`, rather than outside.
junrushao pushed a commit to junrushao/tvm that referenced this pull request Jul 27, 2023
* [TIR] Call TVMBackendFreeWorkspace inside LetStmt

Prior to this commit, the call to `TVMBackendFreeWorkspace` occurred
outside the `LetStmt` that defined the workspace pointer.  While works
with current codegen, as the code produced for `LetStmt` does not
check for out-of-scope access, this access of an out-of-scope should
be avoided.

This commit updates `LowerTVMBuiltin` to produce the call to
`TVMBackendFreeWorkspace` at the end of the `LetStmt`'s body, rather
than just after the `LetStmt`.

* [TIR] Output AttrStmt "storage_alignment" inside the var binding

Prior to this commit, the `AttrStmt` providing the storage alignment
was placed outside the `LetStmt` that defines the variable. As a
result, the alignment assumption is never actually used, as
`CodeGenLLVM::VisitStmt_(const AttrStmtNode*)` only creates an
alignment assumption for in-scope variables.

This commit moves the storage alignment `AttrStmt` to be inside the
`LetStmt`, rather than outside.
junrushao pushed a commit to junrushao/tvm that referenced this pull request Jul 30, 2023
* [TIR] Call TVMBackendFreeWorkspace inside LetStmt

Prior to this commit, the call to `TVMBackendFreeWorkspace` occurred
outside the `LetStmt` that defined the workspace pointer.  While works
with current codegen, as the code produced for `LetStmt` does not
check for out-of-scope access, this access of an out-of-scope should
be avoided.

This commit updates `LowerTVMBuiltin` to produce the call to
`TVMBackendFreeWorkspace` at the end of the `LetStmt`'s body, rather
than just after the `LetStmt`.

* [TIR] Output AttrStmt "storage_alignment" inside the var binding

Prior to this commit, the `AttrStmt` providing the storage alignment
was placed outside the `LetStmt` that defines the variable. As a
result, the alignment assumption is never actually used, as
`CodeGenLLVM::VisitStmt_(const AttrStmtNode*)` only creates an
alignment assumption for in-scope variables.

This commit moves the storage alignment `AttrStmt` to be inside the
`LetStmt`, rather than outside.
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