wrongtest commented on a change in pull request #10582:
URL: https://github.com/apache/tvm/pull/10582#discussion_r825561127
##########
File path: src/target/llvm/codegen_llvm.cc
##########
@@ -1249,15 +1249,14 @@ llvm::Value* CodeGenLLVM::VisitExpr_(const SelectNode*
op) {
llvm::Value* CodeGenLLVM::VisitExpr_(const LetNode* op) {
auto it = let_binding_.find(op->var);
if (it != let_binding_.end()) {
- ICHECK(deep_equal_(it->second->value, op->value))
+ ICHECK(deep_equal_(it->second, op->value))
<< "Let cannot bind the same var to two different values";
} else {
- let_binding_[op->var] = op;
+ let_binding_[op->var] = op->value;
}
auto var_value = MakeValue(op->value);
var_map_[op->var.get()] = var_value;
var_value->setName(op->var->name_hint.c_str());
- analyzer_->Bind(op->var, op->value);
Review comment:
`Bind` means 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
