From ac18d6ab717dcc68744178dd2c6d798cf51cc36e Mon Sep 17 00:00:00 2001 From: root <26priya11@gmail.com> Date: Wed, 13 Mar 2024 05:40:37 +0000 Subject: [PATCH 1/9] evaluater refactor --- csrc/expr_evaluator.cpp | 16 ++++++---------- csrc/expr_evaluator.h | 4 ++++ csrc/ir/base_nodes.cpp | 16 ++++++++++++++++ csrc/ir/base_nodes.h | 4 ++++ 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/csrc/expr_evaluator.cpp b/csrc/expr_evaluator.cpp index 3943c07ab82..bcb58353986 100644 --- a/csrc/expr_evaluator.cpp +++ b/csrc/expr_evaluator.cpp @@ -199,6 +199,11 @@ PolymorphicValue ExpressionEvaluator::evaluate(const Val* value) const { return evaluateHelper(value, known_values); } +PolymorphicValue ExpressionEvaluator::evaluate(const Val* value, + std::unordered_map& known_values) const { + return evaluateHelper(value, known_values); +} + const PolymorphicValue& ExpressionEvaluator::evaluateHelper( const Val* value, std::unordered_map& known_values) const { @@ -213,16 +218,7 @@ const PolymorphicValue& ExpressionEvaluator::evaluateHelper( if (!maybe_concrete_value.get().hasValue()) { if (auto def = value->definition()) { FUSER_PERF_SCOPE("ExpressionEvaluator::evaluate"); - std::vector inputs; - inputs.reserve(def->inputs().size()); - for (auto i : def->inputs()) { - const auto& eval_i = evaluateHelper(i, known_values); - if (!eval_i.hasValue()) { - return null_; - } - inputs.emplace_back(eval_i); - } - auto outputs = def->evaluate(*this, inputs); + auto outputs = def->evaluate(*this, known_values); for (auto i : c10::irange(def->outputs().size())) { known_values[def->output(i)] = std::move(outputs[i]); } diff --git a/csrc/expr_evaluator.h b/csrc/expr_evaluator.h index 28f4cb2288c..d8be0c7dd0b 100644 --- a/csrc/expr_evaluator.h +++ b/csrc/expr_evaluator.h @@ -59,6 +59,10 @@ class ExpressionEvaluator { //! Try to evaluate a value using const evaluator ref NVF_API PolymorphicValue evaluate(const Val* value) const; + PolymorphicValue evaluate( + const Val* value, + std::unordered_map& known_values) const; + bool isKnown(const Val* value) const { return known_values_.count(value) > 0; } diff --git a/csrc/ir/base_nodes.cpp b/csrc/ir/base_nodes.cpp index 94779496564..7fbd90d3dcb 100644 --- a/csrc/ir/base_nodes.cpp +++ b/csrc/ir/base_nodes.cpp @@ -401,6 +401,22 @@ std::vector Expr::evaluate( "Please override the evaluate method"); } +std::vector Expr::evaluate( + const ExpressionEvaluator& ee, + std::unordered_map& known_values) const { + + std::vector expr_inputs; + expr_inputs.reserve(inputs().size()); + for (auto i : inputs()) { + const auto& eval_i = ee.evaluate(i, known_values); + if (!eval_i.hasValue()) { + return {std::monostate{}}; + } + expr_inputs.emplace_back(eval_i); + } + return this->evaluate(ee, expr_inputs); +} + void Expr::addDataAttribute(PolymorphicValue attr) { addAttribute(IrBuilder::create(container(), std::move(attr))); } diff --git a/csrc/ir/base_nodes.h b/csrc/ir/base_nodes.h index a705791252f..b5aa907abc5 100644 --- a/csrc/ir/base_nodes.h +++ b/csrc/ir/base_nodes.h @@ -524,6 +524,10 @@ class NVF_API Expr : public Statement { const ExpressionEvaluator& ee, const std::vector& inputs) const; + virtual std::vector evaluate( + const ExpressionEvaluator& ee, + std::unordered_map& known_values) const; + // Input/output accessors const auto& inputs() const { return inputs_; From 7b6bf7b25c3768f01fc4c70196623e3f917bbff1 Mon Sep 17 00:00:00 2001 From: root <26priya11@gmail.com> Date: Wed, 13 Mar 2024 06:14:08 +0000 Subject: [PATCH 2/9] add null_ --- csrc/ir/base_nodes.cpp | 6 +++--- csrc/ir/base_nodes.h | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/csrc/ir/base_nodes.cpp b/csrc/ir/base_nodes.cpp index 7fbd90d3dcb..b0e82e78c6f 100644 --- a/csrc/ir/base_nodes.cpp +++ b/csrc/ir/base_nodes.cpp @@ -407,10 +407,10 @@ std::vector Expr::evaluate( std::vector expr_inputs; expr_inputs.reserve(inputs().size()); - for (auto i : inputs()) { - const auto& eval_i = ee.evaluate(i, known_values); + for (auto inp : inputs()) { + const auto& eval_i = ee.evaluate(inp, known_values); if (!eval_i.hasValue()) { - return {std::monostate{}}; + return {null_}; } expr_inputs.emplace_back(eval_i); } diff --git a/csrc/ir/base_nodes.h b/csrc/ir/base_nodes.h index b5aa907abc5..8ac03d60c2e 100644 --- a/csrc/ir/base_nodes.h +++ b/csrc/ir/base_nodes.h @@ -636,6 +636,8 @@ class NVF_API Expr : public Statement { std::vector attributes_; + PolymorphicValue null_ = std::monostate{}; + private: std::vector inputs_; std::vector outputs_; From 84bfda1289632e5b8c0df0311fe9cb078b31b56b Mon Sep 17 00:00:00 2001 From: root <26priya11@gmail.com> Date: Wed, 13 Mar 2024 06:16:45 +0000 Subject: [PATCH 3/9] catop evaluate --- csrc/ir/internal_nodes.h | 2 +- csrc/ir/nodes.cpp | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/csrc/ir/internal_nodes.h b/csrc/ir/internal_nodes.h index e2ac73720da..75d33c0d65a 100644 --- a/csrc/ir/internal_nodes.h +++ b/csrc/ir/internal_nodes.h @@ -2240,7 +2240,7 @@ class NVF_API CatOp : public Expr { std::string toInlineString(int indent_size = 0) const override; std::vector evaluate( const ExpressionEvaluator& ee, - const std::vector& inputs) const override; + std::unordered_map& known_values) const override; int64_t concatenatedDim() const { return attribute(0); diff --git a/csrc/ir/nodes.cpp b/csrc/ir/nodes.cpp index ff4ccf09894..2a8b9cd887c 100644 --- a/csrc/ir/nodes.cpp +++ b/csrc/ir/nodes.cpp @@ -4459,14 +4459,14 @@ Val* CatOp::getPred(int input_idx) const { std::vector CatOp::evaluate( const ExpressionEvaluator& ee, - const std::vector& inputs) const { - std::vector in; + std::unordered_map& known_values) const { + std::vector unpadded_inputs; int64_t concat_dim = concatenatedDim(); - for (auto i : c10::irange(inputs.size())) { - auto unpadded_inp = ee.evaluate(input(i)->definition()->input(0)); - in.push_back(unpadded_inp.as()); + for (auto inp : inputs()) { + auto eval_i = ee.evaluate(inp->definition()->input(0), known_values); + unpadded_inputs.push_back(eval_i.as()); } - return {at::cat(in, concat_dim)}; + return {at::cat(unpadded_inputs, concat_dim)}; } } // namespace nvfuser From d9023d59eb3e80236e2fd2acd279925832c2ff1a Mon Sep 17 00:00:00 2001 From: root <26priya11@gmail.com> Date: Wed, 13 Mar 2024 06:59:47 +0000 Subject: [PATCH 4/9] add test for catop --- test/test_evaluator.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/test_evaluator.cpp b/test/test_evaluator.cpp index eb0978bc9fa..20cc39f49b7 100644 --- a/test/test_evaluator.cpp +++ b/test/test_evaluator.cpp @@ -695,4 +695,33 @@ TEST_F(ExprEvalTest, SumDiv) { evaluator.evaluate(out); } +// Verify that the padded inputs are not evaluated +TEST_F(ExprEvalTest, CatOp) { + Fusion fusion; + FusionGuard fg(&fusion); + + auto tv0 = makeContigTensor(2); + auto tv1 = makeContigTensor(2); + auto tv2 = cat({tv0, tv1}, 0); + fusion.addInput(tv0); + fusion.addInput(tv1); + fusion.addOutput(tv2); + + auto options = at::TensorOptions().dtype(at::kFloat).device(at::kCUDA, 0); + auto t0 = at::randn({3, 2}, options); + auto t1 = at::randn({3, 2}, options); + + ExpressionEvaluator evaluator; + evaluator.bind(tv0, t0); + evaluator.bind(tv1, t1); + + at::Tensor out = evaluator.evaluate(tv2).as(); + + for (auto padded_in : tv2->definition()->inputs()) { + EXPECT_FALSE(evaluator.isKnown(padded_in)); + } + + EXPECT_TRUE(at::equal(out, at::cat({t0, t1}, 0))); +} + } // namespace nvfuser From c8540756d4076d06ebd23b252647f8b3c9ec483e Mon Sep 17 00:00:00 2001 From: root <26priya11@gmail.com> Date: Wed, 13 Mar 2024 18:58:04 +0000 Subject: [PATCH 5/9] comment, lintrunner --- csrc/expr_evaluator.cpp | 5 +++-- csrc/expr_evaluator.h | 6 ++++-- csrc/ir/base_nodes.cpp | 3 +-- csrc/ir/internal_nodes.h | 3 ++- csrc/ir/nodes.cpp | 2 ++ 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/csrc/expr_evaluator.cpp b/csrc/expr_evaluator.cpp index bcb58353986..55db7567a18 100644 --- a/csrc/expr_evaluator.cpp +++ b/csrc/expr_evaluator.cpp @@ -199,8 +199,9 @@ PolymorphicValue ExpressionEvaluator::evaluate(const Val* value) const { return evaluateHelper(value, known_values); } -PolymorphicValue ExpressionEvaluator::evaluate(const Val* value, - std::unordered_map& known_values) const { +PolymorphicValue ExpressionEvaluator::evaluate( + const Val* value, + std::unordered_map& known_values) const { return evaluateHelper(value, known_values); } diff --git a/csrc/expr_evaluator.h b/csrc/expr_evaluator.h index d8be0c7dd0b..a9ea5a7ff49 100644 --- a/csrc/expr_evaluator.h +++ b/csrc/expr_evaluator.h @@ -59,9 +59,11 @@ class ExpressionEvaluator { //! Try to evaluate a value using const evaluator ref NVF_API PolymorphicValue evaluate(const Val* value) const; + //! Used by Expr::evaluate when evaluating inputs to properly update the + //! known_values. PolymorphicValue evaluate( - const Val* value, - std::unordered_map& known_values) const; + const Val* value, + std::unordered_map& known_values) const; bool isKnown(const Val* value) const { return known_values_.count(value) > 0; diff --git a/csrc/ir/base_nodes.cpp b/csrc/ir/base_nodes.cpp index b0e82e78c6f..53c9ff114cf 100644 --- a/csrc/ir/base_nodes.cpp +++ b/csrc/ir/base_nodes.cpp @@ -404,7 +404,6 @@ std::vector Expr::evaluate( std::vector Expr::evaluate( const ExpressionEvaluator& ee, std::unordered_map& known_values) const { - std::vector expr_inputs; expr_inputs.reserve(inputs().size()); for (auto inp : inputs()) { @@ -414,7 +413,7 @@ std::vector Expr::evaluate( } expr_inputs.emplace_back(eval_i); } - return this->evaluate(ee, expr_inputs); + return this->evaluate(ee, expr_inputs); } void Expr::addDataAttribute(PolymorphicValue attr) { diff --git a/csrc/ir/internal_nodes.h b/csrc/ir/internal_nodes.h index 75d33c0d65a..5cf9dc952c5 100644 --- a/csrc/ir/internal_nodes.h +++ b/csrc/ir/internal_nodes.h @@ -2240,7 +2240,8 @@ class NVF_API CatOp : public Expr { std::string toInlineString(int indent_size = 0) const override; std::vector evaluate( const ExpressionEvaluator& ee, - std::unordered_map& known_values) const override; + std::unordered_map& known_values) + const override; int64_t concatenatedDim() const { return attribute(0); diff --git a/csrc/ir/nodes.cpp b/csrc/ir/nodes.cpp index 2a8b9cd887c..56f0acf604e 100644 --- a/csrc/ir/nodes.cpp +++ b/csrc/ir/nodes.cpp @@ -4460,6 +4460,8 @@ Val* CatOp::getPred(int input_idx) const { std::vector CatOp::evaluate( const ExpressionEvaluator& ee, std::unordered_map& known_values) const { + // CatOp is preceded by a PadOp internally. + // For ATen evaluation, directly compute the unpadded inputs. std::vector unpadded_inputs; int64_t concat_dim = concatenatedDim(); for (auto inp : inputs()) { From 09f21a829fca1e07b531933708467ee8f60676e9 Mon Sep 17 00:00:00 2001 From: root <26priya11@gmail.com> Date: Wed, 13 Mar 2024 20:35:41 +0000 Subject: [PATCH 6/9] unify evaluateHelper and evaluate --- csrc/expr_evaluator.cpp | 12 +++--------- csrc/expr_evaluator.h | 8 ++------ 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/csrc/expr_evaluator.cpp b/csrc/expr_evaluator.cpp index 55db7567a18..0b7b6875ec6 100644 --- a/csrc/expr_evaluator.cpp +++ b/csrc/expr_evaluator.cpp @@ -191,21 +191,15 @@ const PolymorphicValue& ExpressionEvaluator::evaluate(ParallelType pt) { } const PolymorphicValue& ExpressionEvaluator::evaluate(const Val* value) { - return evaluateHelper(value, known_values_); + return evaluate(value, known_values_); } PolymorphicValue ExpressionEvaluator::evaluate(const Val* value) const { std::unordered_map known_values; - return evaluateHelper(value, known_values); + return evaluate(value, known_values); } -PolymorphicValue ExpressionEvaluator::evaluate( - const Val* value, - std::unordered_map& known_values) const { - return evaluateHelper(value, known_values); -} - -const PolymorphicValue& ExpressionEvaluator::evaluateHelper( +const PolymorphicValue& ExpressionEvaluator::evaluate( const Val* value, std::unordered_map& known_values) const { if (precomputed_values_ && precomputed_values_->ready()) { diff --git a/csrc/expr_evaluator.h b/csrc/expr_evaluator.h index a9ea5a7ff49..a5cd069e56c 100644 --- a/csrc/expr_evaluator.h +++ b/csrc/expr_evaluator.h @@ -59,9 +59,8 @@ class ExpressionEvaluator { //! Try to evaluate a value using const evaluator ref NVF_API PolymorphicValue evaluate(const Val* value) const; - //! Used by Expr::evaluate when evaluating inputs to properly update the - //! known_values. - PolymorphicValue evaluate( + //! Base evaluate method called by other overloads and Expr::evaluate + const PolymorphicValue& evaluate( const Val* value, std::unordered_map& known_values) const; @@ -94,9 +93,6 @@ class ExpressionEvaluator { const Val* value, const std::unordered_map& additional_known_values) const; - const PolymorphicValue& evaluateHelper( - const Val* value, - std::unordered_map& known_values) const; private: // TODO: Consider make this const. It can't be const as bind() of From 32a2e887e08d6a1c393a3501540cf334aae6c1a1 Mon Sep 17 00:00:00 2001 From: root <26priya11@gmail.com> Date: Wed, 13 Mar 2024 21:34:41 +0000 Subject: [PATCH 7/9] review comments --- csrc/expr_evaluator.h | 7 ++++--- csrc/ir/base_nodes.cpp | 2 +- csrc/ir/base_nodes.h | 9 +++++++-- csrc/ir/nodes.cpp | 6 +++++- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/csrc/expr_evaluator.h b/csrc/expr_evaluator.h index a5cd069e56c..bd8484c06e0 100644 --- a/csrc/expr_evaluator.h +++ b/csrc/expr_evaluator.h @@ -56,10 +56,12 @@ class ExpressionEvaluator { //! Try to evaluate a parallel dimension const PolymorphicValue& evaluate(ParallelType pt); - //! Try to evaluate a value using const evaluator ref + //! Evaluates a value through a const evaluator reference. + //! Initializes a known_values map to store intermediate values in lieu of + //! known_values_. NVF_API PolymorphicValue evaluate(const Val* value) const; - //! Base evaluate method called by other overloads and Expr::evaluate + //! Base evaluate method called by other overloads and Expr::evaluate. const PolymorphicValue& evaluate( const Val* value, std::unordered_map& known_values) const; @@ -104,7 +106,6 @@ class ExpressionEvaluator { PrecomputedValues* precomputed_values_ = nullptr; std::unordered_map known_values_; std::unordered_map known_named_scalars_; - PolymorphicValue null_ = std::monostate{}; }; } // namespace nvfuser diff --git a/csrc/ir/base_nodes.cpp b/csrc/ir/base_nodes.cpp index 53c9ff114cf..a54a5f0d2c2 100644 --- a/csrc/ir/base_nodes.cpp +++ b/csrc/ir/base_nodes.cpp @@ -409,7 +409,7 @@ std::vector Expr::evaluate( for (auto inp : inputs()) { const auto& eval_i = ee.evaluate(inp, known_values); if (!eval_i.hasValue()) { - return {null_}; + return {std::monostate{}}; } expr_inputs.emplace_back(eval_i); } diff --git a/csrc/ir/base_nodes.h b/csrc/ir/base_nodes.h index 8ac03d60c2e..46ed9be13c5 100644 --- a/csrc/ir/base_nodes.h +++ b/csrc/ir/base_nodes.h @@ -524,6 +524,13 @@ class NVF_API Expr : public Statement { const ExpressionEvaluator& ee, const std::vector& inputs) const; + // This version allows evaluation of multiple ops together instead of one op + // at a time by overriding and skipping computation of intermediate inputs + // that are not required. For example: 1. CatOp is internally preceded by + // PadOp but the ATen evaluation uses only the unpadded inputs + // and the evaluation of padded inputs can be skipped. + // 2. Evaluating patterns in matmul fallback such as MmaOp + + // Cast/ MmaOp + Bias + Cast virtual std::vector evaluate( const ExpressionEvaluator& ee, std::unordered_map& known_values) const; @@ -636,8 +643,6 @@ class NVF_API Expr : public Statement { std::vector attributes_; - PolymorphicValue null_ = std::monostate{}; - private: std::vector inputs_; std::vector outputs_; diff --git a/csrc/ir/nodes.cpp b/csrc/ir/nodes.cpp index 56f0acf604e..d8ae0b00ae1 100644 --- a/csrc/ir/nodes.cpp +++ b/csrc/ir/nodes.cpp @@ -4463,8 +4463,12 @@ std::vector CatOp::evaluate( // CatOp is preceded by a PadOp internally. // For ATen evaluation, directly compute the unpadded inputs. std::vector unpadded_inputs; + unpadded_inputs.reserve(inputs().size()); int64_t concat_dim = concatenatedDim(); - for (auto inp : inputs()) { + for (Val* inp : inputs()) { + NVF_CHECK( + inp->definition() != nullptr && inp->definition()->isA(), + "Expected CatOp to be preceded by a PadOp."); auto eval_i = ee.evaluate(inp->definition()->input(0), known_values); unpadded_inputs.push_back(eval_i.as()); } From ebb6a5e667503898fa2edddc813c752028ab7f11 Mon Sep 17 00:00:00 2001 From: root <26priya11@gmail.com> Date: Wed, 13 Mar 2024 22:03:17 +0000 Subject: [PATCH 8/9] review comments --- csrc/expr_evaluator.h | 1 + 1 file changed, 1 insertion(+) diff --git a/csrc/expr_evaluator.h b/csrc/expr_evaluator.h index bd8484c06e0..f2a952e95f7 100644 --- a/csrc/expr_evaluator.h +++ b/csrc/expr_evaluator.h @@ -106,6 +106,7 @@ class ExpressionEvaluator { PrecomputedValues* precomputed_values_ = nullptr; std::unordered_map known_values_; std::unordered_map known_named_scalars_; + PolymorphicValue null_ = std::monostate{}; }; } // namespace nvfuser From f982f89b6d97a7422cb45ac3e8473307b4daf288 Mon Sep 17 00:00:00 2001 From: root <26priya11@gmail.com> Date: Thu, 14 Mar 2024 22:35:21 +0000 Subject: [PATCH 9/9] fix indentation --- csrc/ir/base_nodes.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/csrc/ir/base_nodes.h b/csrc/ir/base_nodes.h index 46ed9be13c5..1862726946d 100644 --- a/csrc/ir/base_nodes.h +++ b/csrc/ir/base_nodes.h @@ -526,11 +526,11 @@ class NVF_API Expr : public Statement { // This version allows evaluation of multiple ops together instead of one op // at a time by overriding and skipping computation of intermediate inputs - // that are not required. For example: 1. CatOp is internally preceded by - // PadOp but the ATen evaluation uses only the unpadded inputs - // and the evaluation of padded inputs can be skipped. - // 2. Evaluating patterns in matmul fallback such as MmaOp + - // Cast/ MmaOp + Bias + Cast + // that are not required. For example: + // 1. CatOp is internally preceded by PadOp but the ATen evaluation uses only + // the unpadded inputs and the evaluation of padded inputs can be skipped. + // 2. Evaluating patterns in matmul fallback such as MmaOp + Cast/ MmaOp + + // Bias + Cast virtual std::vector evaluate( const ExpressionEvaluator& ee, std::unordered_map& known_values) const;