Skip to content

Commit

Permalink
[Codegen] Swap out analyzer when outlining (apache#9117)
Browse files Browse the repository at this point in the history
Problem: the `analyzer_` in `CodeGenLLVM` and derived classes
can generate invalid code for outlined functions.

Consider code like this:

  let x = y in
    // attr compute_scope
    blah = x

Then it gets outlined in codegen_cpu (for example):

  let x = y in
    call foo(x)

  foo(x) {
    blah = x
  }

Now, if `analyzer_->Simplify` was run on the body of `foo`, it
would produce:

  foo(x) {
    blah = y
  }

Because the `analyzer_` knows that `x` is same as `y` (because
of the `Let` statemement), but doesn't know that `y` is no longer
available in the outlined function `foo`.

See
https://discuss.tvm.apache.org/t/compute-scope-issue-with-analyzer-invalid-simplification/11111
  • Loading branch information
Krzysztof Parzyszek authored and ylc committed Sep 29, 2021
1 parent f93c648 commit ab1e998
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions src/target/llvm/codegen_cpu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -473,14 +473,17 @@ void CodeGenCPU::CreateComputeScope(const AttrStmtNode* op) {
}
#endif
}
auto new_analyzer = std::make_unique<arith::Analyzer>();
std::swap(function_, fcompute);
std::swap(new_vmap, var_map_);
std::swap(analyzer_, new_analyzer);
std::swap(var_map_, new_vmap);
BasicBlock* compute_entry = BasicBlock::Create(*ctx_, "entry", function_);
builder_->SetInsertPoint(compute_entry);
this->VisitStmt(op->body);
builder_->CreateRet(ConstInt32(0));
// swap the var map back, now we are back on track.
std::swap(new_vmap, var_map_);
std::swap(var_map_, new_vmap);
std::swap(analyzer_, new_analyzer);
std::swap(function_, fcompute);
builder_->SetInsertPoint(compute_call_end);
}
Expand Down Expand Up @@ -551,13 +554,16 @@ void CodeGenCPU::CreateParallelLaunch(const Stmt& body, int num_task) {
new_vmap[par_env.num_task.get()] =
builder_->CreateLoad(builder_->CreateInBoundsGEP(penv, {ConstInt32(0), ConstInt32(1)}));
par_env.penv = penv;
auto new_analyzer = std::make_unique<arith::Analyzer>();
std::swap(function_, f);
std::swap(parallel_env_, par_env);
std::swap(analyzer_, new_analyzer);
std::swap(var_map_, new_vmap);
this->VisitStmt(body);
builder_->CreateRet(ConstInt32(0));
// swap the var map back, now we are back on track.
std::swap(var_map_, new_vmap);
std::swap(analyzer_, new_analyzer);
std::swap(parallel_env_, par_env);
std::swap(function_, f);
ICHECK_NE(par_env.parallel_loop_count, 0) << "Cannot find parallel loop within parallel launch";
Expand Down Expand Up @@ -604,12 +610,15 @@ void CodeGenCPU::CreateStaticInit(const std::string& init_fname, const Stmt& bod
std::unordered_map<const VarNode*, llvm::Value*> new_vmap;
UnpackClosureData(cdata, vfields, &new_vmap);
ICHECK(parallel_env_.penv == nullptr);
auto new_analyzer = std::make_unique<arith::Analyzer>();
std::swap(function_, f);
std::swap(analyzer_, new_analyzer);
std::swap(var_map_, new_vmap);
this->VisitStmt(body);
builder_->CreateRet(ConstInt32(0));
// swap the var map back, now we are back on track.
std::swap(var_map_, new_vmap);
std::swap(analyzer_, new_analyzer);
std::swap(function_, f);
builder_->SetInsertPoint(init_end);
}
Expand Down

0 comments on commit ab1e998

Please sign in to comment.