-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[TIR][CodeGen] Process buffer elem_offset in target codegen #10582
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
Changes from all commits
a1be7fe
6191e48
8915fbc
468fed1
61c4d9c
82d62dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -439,7 +439,7 @@ class CodeGenLLVM : public ExprFunctor<llvm::Value*(const PrimExpr&)>, | |
| // deep comparison of PrimExpr | ||
| ExprDeepEqual deep_equal_; | ||
| // binding of let variables. Enables duplicate var defs that map to same value | ||
| std::unordered_map<Var, const LetNode*, ObjectPtrHash, ObjectPtrEqual> let_binding_; | ||
| std::unordered_map<Var, PrimExpr, ObjectPtrHash, ObjectPtrEqual> let_binding_; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please explain this change?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A local expression can be visited (eg, a simplify res) and some pointer are recorded. But when the local scope ends the backing object is expired. |
||
| // Cache potential common path ops to slightly improve lookup time. | ||
| // global symbol table. | ||
| OpAttrMap<TGlobalSymbol> op_attr_global_symbol_ = Op::GetAttrMap<TGlobalSymbol>("TGlobalSymbol"); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we need
bindhere?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bindmeans analyzer will always expand the value expression for simplify and other functionalities. It will break the evaluation order specified by lets, which should be respsected in codegen phase. The issue could be triggered on existing testcases by the simplify this PR adds.I can use a local analyzer on this PR's purpose but I think this is still an issue to resolve.