NVIDIA/Fuser

Clarify container methods for collections of statements

Open

#2,949 opened on Sep 17, 2024

View on GitHub
 (4 comments) (2 reactions) (0 assignees)C++ (80 forks)github user discovery
cleanupgood first issue

Repository metrics

Stars
 (392 stars)
PR merge metrics
 (PR metrics pending)

Description

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.

Contributor guide