[668]: fixes tests, removes seqEwDiv which is not suitable for pushDown in integer space, adds docs to tryPushDown

Signed-off-by: Maximilian Ehlers <daphnevm@sodawa.com>

Author: Maximilian Ehlers

src/ir/daphneir/Canonicalize.cpp

@@ -17,6 +17,7 @@
 #include "ir/daphneir/Daphne.h"
 #include "ir/daphneir/DaphnePushDownTraits.h"
 #include "mlir/Dialect/SCF/IR/SCF.h"
+#include "mlir/IR/Builders.h"
 #include "mlir/Support/LogicalResult.h"
 #include <compiler/utils/CompilerUtils.h>
 #include <util/DaphneLogger.h>
@@ -329,7 +330,8 @@ template <class Operation> bool pushDownBinary(Operation op, mlir::PatternRewrit
         auto fillValue = lhsFill.getArg();
         auto height = lhsFill.getNumRows();
         auto width = lhsFill.getNumCols();
-        auto newOp = rewriter.create<Operation>(op.getLoc(), fillValue, rhs);
+        auto newOp = rewriter.create<Operation>(op.getLoc(), CompilerUtils::getValueType(op.getResult().getType()),
+                                                fillValue, rhs);
         auto newCombinedOpAfterPushDown =
             rewriter.create<mlir::daphne::FillOp>(op.getLoc(), op.getResult().getType(), newOp, height, width);
         rewriter.replaceOp(op, {newCombinedOpAfterPushDown});
@@ -363,13 +365,14 @@ template <class Operation> bool pushDownBinary(Operation op, mlir::PatternRewrit
         auto newTo = rewriter.create<Operation>(op.getLoc(), to, rhs);
 
         if (supportsPushDownWithIntervalUpdate) {
-            mlir::daphne::EwMulOp newInc = rewriter.create<mlir::daphne::EwMulOp>(op.getLoc(), inc, rhs);
+            auto newInc = rewriter.create<Operation>(op.getLoc(), rhs, inc);
+
             auto newCombinedOpAfterPushDown =
                 rewriter.create<mlir::daphne::SeqOp>(op.getLoc(), op.getResult().getType(), newFrom, newTo, newInc);
             rewriter.replaceOp(op, {newCombinedOpAfterPushDown});
             return true;
-        } else {
 
+        } else {
             auto newCombinedOpAfterPushDown =
                 rewriter.create<mlir::daphne::SeqOp>(op.getLoc(), op.getResult().getType(), newFrom, newTo, inc);
             rewriter.replaceOp(op, {newCombinedOpAfterPushDown});
@@ -379,6 +382,29 @@ template <class Operation> bool pushDownBinary(Operation op, mlir::PatternRewrit
     return false;
 }
 
+/**
+ * @brief PushDown optimizations will try to optimize aways intermediate results
+ * when function results are used with arithmetic operations.
+ * An example is `fill(2,2,3) + 3` which will evaluate to
+ * 3,3
+ * 3,3
+ *
+ * and the applies +3 to each element of the intermediate individually leading to
+ *
+ * 6,6
+ * 6,6
+ *
+ * Instead we can push the math inside the function and have it evaluate first:
+ *
+ * fill(2,2,3 + 3) -> fill(2,2,6)
+ *
+ * which directly fills the matrix
+ *
+ * 6,6
+ * 6,6
+ *
+ * and skips the intermediate.
+ */
 template <class Operation> bool tryPushDown(Operation op, mlir::PatternRewriter &rewriter) {
     if constexpr (std::is_same<Operation, mlir::daphne::EwAbsOp>() ||
                   std::is_same<Operation, mlir::daphne::EwExpOp>() || std::is_same<Operation, mlir::daphne::EwLnOp>() ||

src/ir/daphneir/DaphneOps.td

@@ -362,7 +362,7 @@ def Daphne_EwSubOp    : Daphne_EwBinaryOp<"ewSub", NumScalar, [ValueTypeFromArgs
 def Daphne_EwMulOp    : Daphne_EwBinaryOp<"ewMul", NumScalar, [ValueTypeFromArgs, CastArgsToResType, Commutative, EwSparseIfEither, CUDASupport, PushDown, PushDownLinear, PushDownWithIntervalUpdate]> {
     let hasCanonicalizeMethod = 1;
 }
-def Daphne_EwDivOp    : Daphne_EwBinaryOp<"ewDiv", NumScalar, [ValueTypeFromArgs, CastArgsToResType, CUDASupport, PushDown, PushDownLinear, PushDownWithIntervalUpdate]> {
+def Daphne_EwDivOp    : Daphne_EwBinaryOp<"ewDiv", NumScalar, [ValueTypeFromArgs, CastArgsToResType, CUDASupport, PushDown, PushDownLinear]> {
     let hasCanonicalizeMethod = 1;
 }
 def Daphne_EwPowOp    : Daphne_EwBinaryOp<"ewPow", NumScalar, [ValueTypeFromArgs, CastArgsToResType, CUDASupport, PushDown]> {

test/api/cli/operations/CanonicalizationPushDownTest.cpp

@@ -53,7 +53,6 @@ MAKE_TEST_CASE("randEwSub", 1)
 MAKE_TEST_CASE("randEwMul", 1)
 MAKE_TEST_CASE("randEwDiv", 1)
 MAKE_TEST_CASE("randEwAbs", 2)
-MAKE_TEST_CASE("seqEwAdd", 1)
-MAKE_TEST_CASE("seqEwSub", 1)
-MAKE_TEST_CASE("seqEwMul", 1)
-MAKE_TEST_CASE("seqEwDiv", 1)
+MAKE_TEST_CASE("seqEwAdd", 2)
+MAKE_TEST_CASE("seqEwSub", 2)
+MAKE_TEST_CASE("seqEwMul", 2)

test/api/cli/operations/randEwAbs_2.daphne

@@ -1,4 +1,4 @@
-randIntermediate = rand(2,2,-3,1,1.0,7);
+randIntermediate = rand(2,2,-3,-1,1.0,7);
 result = abs(randIntermediate);
 
 print(result);

test/api/cli/operations/randEwAbs_2.txt

@@ -1,3 +1,3 @@
 DenseMatrix(2x2, int64_t)
-3 2
-2 1
+1 1
+3 1

test/api/cli/operations/seqEwAdd_2.daphne

@@ -0,0 +1,3 @@
+result = seq(1,5) + 2;
+
+print(result);

test/api/cli/operations/seqEwAdd_2.txt

@@ -0,0 +1,6 @@
+DenseMatrix(5x1, int64_t)
+3
+4
+5
+6
+7

test/api/cli/operations/seqEwDiv_1.daphne

@@ -0,0 +1,3 @@
+result = seq(1,5) * 2;
+
+print(result);