NVIDIA/Fuser

Clarify container methods for collections of statements

Open

#2,949 创建于 2024年9月17日

在 GitHub 查看
 (4 评论) (2 反应) (0 负责人)C++ (80 fork)github user discovery
cleanupgood first issue

仓库指标

Star
 (392 star)
PR 合并指标
 (PR 指标待抓取)

描述

We currently have the following methods available in a Fusion object:

  • deque<Val*> IrContainer::deterministic_vals(): returns Vals in insertion order, not in topological order and not just those leading to outputs (since IrContainer doesn't track inputs or outputs)
  • deque<Expr*> IrContainer::deterministic_exprs(): similar to above but for Exprs.
  • const std::unordered_set<Expr*>& IrContainer::unordered_exprs(): return all Exprs in no particular order. Again this gives all expressions regardless of outputs.
  • const std::unordered_set<Val*>& IrContainer::vals(): same as above but for Val. Note the name is inconsistent
  • std::vector<Expr*> Fusion::exprs(): returns a vector of topologically sorted expressions leading to outputs.
  • std::vector<Val*> Fusion::inputsOf(Val* val): comment says this will "Return a vector of fusion inputs that feed this Val", but what it really returns is all producer Vals that don't themselves have definitions. val->isFusionInput() is not checked, so this could be a method of IrContainer instead..
  • std::vector<Val*> Fusion::usedMathVals(): return all outputs and Vals leading to outputs, and all siblings of any of those Vals (sibling means another output from the same multi-output expression).
  • std::vector<Val*> Fusion::terminatingMathVals(): filter usedMathVals() to find vals that have definitions but no uses (i.e. whether they are outputs or not, no other output consumes them).

I came to this because I was confused thinking fusion->vals() would behave similarly to fusion->exprs(), which it does not. I think we should do at least the following cleanups:

  • Fix the comment of Fusion::inputsOf to accurately reflect what it does.
  • Move Fusion::inputsOf to IrContainer.
  • Rename IrContainer::vals() to IrContainer::unordered_vals().
  • Rename Fusion::exprs() to Fusion::usedExprs() to reflect that these are only the Exprs that lead to outputs unlike those returned by unordered_exprs().
  • Change snake_case method names like deterministic_vals to camelCase.
  • Consider renaming usedMathVals() to something like producedMathVals(). The interesting thing about this method that it actually returns unused siblings of all the live Vals in the fusion so the name should not suggest that they are all used.

There are probably some other ways to clarify this part of the code that I've missed.

贡献者指南