Repository: systemml Updated Branches: refs/heads/master 259814e6c -> 5b8d62659
[SYSTEMML-1966] Fix codegen plan cache false negatives, cleanups This patch fixes the codegen plan cache to prevent false negatives due to different hash codes that are only based on different data nodes. Furthermore, this also includes some smaller cleanups such as the compilation of mult2 and pow2 in outer templates as well as the exploration of valid fusion plans when using fusion heuristics. Project: http://git-wip-us.apache.org/repos/asf/systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/60ad522e Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/60ad522e Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/60ad522e Branch: refs/heads/master Commit: 60ad522eb4107e55cd9418cc8494208e6c36e28c Parents: 259814e Author: Matthias Boehm <mboe...@gmail.com> Authored: Tue Oct 17 20:16:33 2017 -0700 Committer: Matthias Boehm <mboe...@gmail.com> Committed: Tue Oct 17 21:39:57 2017 -0700 ---------------------------------------------------------------------- .../org/apache/sysml/hops/codegen/SpoofCompiler.java | 12 ++++++++---- .../apache/sysml/hops/codegen/cplan/CNodeData.java | 6 +++--- .../hops/codegen/template/CPlanCSERewriter.java | 2 +- .../hops/codegen/template/TemplateOuterProduct.java | 15 ++++++++++----- .../sysml/hops/codegen/template/TemplateRow.java | 9 ++++++--- .../sysml/runtime/matrix/data/LibMatrixMult.java | 6 +++--- 6 files changed, 31 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/systemml/blob/60ad522e/src/main/java/org/apache/sysml/hops/codegen/SpoofCompiler.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/codegen/SpoofCompiler.java b/src/main/java/org/apache/sysml/hops/codegen/SpoofCompiler.java index 5ff90fb..4af8540 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/SpoofCompiler.java +++ b/src/main/java/org/apache/sysml/hops/codegen/SpoofCompiler.java @@ -139,6 +139,10 @@ public class SpoofCompiler return this == FUSE_ALL || this == FUSE_NO_REDUNDANCY; } + public boolean isCostBased() { + return this == FUSE_COST_BASED_V2 + || this == FUSE_COST_BASED; + } } public enum PlanCachePolicy { @@ -399,14 +403,14 @@ public class SpoofCompiler //explain debug output cplans or generated source code if( LOG.isTraceEnabled() || DMLScript.EXPLAIN.isHopsType(recompile) ) { - LOG.info("Codegen EXPLAIN (generated cplan for HopID: " - + cplan.getKey() + ", line "+tmp.getValue().getBeginLine() + "):"); + LOG.info("Codegen EXPLAIN (generated cplan for HopID: " + cplan.getKey() + + ", line "+tmp.getValue().getBeginLine() + ", hash="+tmp.getValue().hashCode()+"):"); LOG.info(tmp.getValue().getClassname() + Explain.explainCPlan(cplan.getValue().getValue())); } if( LOG.isTraceEnabled() || DMLScript.EXPLAIN.isRuntimeType(recompile) ) { - LOG.info("Codegen EXPLAIN (generated code for HopID: " - + cplan.getKey() + ", line "+tmp.getValue().getBeginLine() + "):"); + LOG.info("Codegen EXPLAIN (generated code for HopID: " + cplan.getKey() + + ", line "+tmp.getValue().getBeginLine() + ", hash="+tmp.getValue().hashCode()+"):"); LOG.info(src); } http://git-wip-us.apache.org/repos/asf/systemml/blob/60ad522e/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java index ce343cf..653b50b 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java +++ b/src/main/java/org/apache/sysml/hops/codegen/cplan/CNodeData.java @@ -106,9 +106,9 @@ public class CNodeData extends CNode public boolean equals(Object o) { return (o instanceof CNodeData && super.equals(o) - && isLiteral() == ((CNodeData)o).isLiteral() - && (isLiteral() || !_strictEquals) ? + && isLiteral() == ((CNode)o).isLiteral() + && ((isLiteral() || !_strictEquals) ? _name.equals(((CNodeData)o)._name) : - _hopID == ((CNodeData)o)._hopID); + _hopID == ((CNodeData)o)._hopID)); } } http://git-wip-us.apache.org/repos/asf/systemml/blob/60ad522e/src/main/java/org/apache/sysml/hops/codegen/template/CPlanCSERewriter.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/codegen/template/CPlanCSERewriter.java b/src/main/java/org/apache/sysml/hops/codegen/template/CPlanCSERewriter.java index 3d12cfe..318e30a 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/template/CPlanCSERewriter.java +++ b/src/main/java/org/apache/sysml/hops/codegen/template/CPlanCSERewriter.java @@ -56,7 +56,7 @@ public class CPlanCSERewriter //step 3: reset data nodes to imprecise comparison tpl.resetVisitStatusOutputs(); for( CNode out : outputs ) - rSetStrictDataNodeComparision(out, true); + rSetStrictDataNodeComparision(out, false); tpl.resetVisitStatusOutputs(); return tpl; http://git-wip-us.apache.org/repos/asf/systemml/blob/60ad522e/src/main/java/org/apache/sysml/hops/codegen/template/TemplateOuterProduct.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateOuterProduct.java b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateOuterProduct.java index f3880b1..256f540 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateOuterProduct.java +++ b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateOuterProduct.java @@ -174,6 +174,7 @@ public class TemplateOuterProduct extends TemplateBase { } else if(hop instanceof BinaryOp) { + BinaryOp bop = (BinaryOp) hop; CNode cdata1 = tmp.get(hop.getInput().get(0).getHopID()); CNode cdata2 = tmp.get(hop.getInput().get(1).getHopID()); String primitiveOpName = ((BinaryOp)hop).getOp().toString(); @@ -186,8 +187,12 @@ public class TemplateOuterProduct extends TemplateBase { //add lookups if required cdata1 = TemplateUtils.wrapLookupIfNecessary(cdata1, hop.getInput().get(0)); cdata2 = TemplateUtils.wrapLookupIfNecessary(cdata2, hop.getInput().get(1)); - - out = new CNodeBinary(cdata1, cdata2, BinType.valueOf(primitiveOpName)); + if( bop.getOp()==OpOp2.POW && cdata2.isLiteral() && cdata2.getVarname().equals("2") ) + out = new CNodeUnary(cdata1, UnaryType.POW2); + else if( bop.getOp()==OpOp2.MULT && cdata2.isLiteral() && cdata2.getVarname().equals("2") ) + out = new CNodeUnary(cdata1, UnaryType.MULT2); + else + out = new CNodeBinary(cdata1, cdata2, BinType.valueOf(primitiveOpName)); } else if(hop instanceof AggBinaryOp) { @@ -213,14 +218,14 @@ public class TemplateOuterProduct extends TemplateBase { //final left/right matrix mult, see close else { if( cdata1.getDataType().isScalar() ) - out = new CNodeBinary(cdata2, cdata1, BinType.VECT_MULT_ADD); + out = new CNodeBinary(cdata2, cdata1, BinType.VECT_MULT_ADD); else - out = new CNodeBinary(cdata1, cdata2, BinType.VECT_MULT_ADD); + out = new CNodeBinary(cdata1, cdata2, BinType.VECT_MULT_ADD); } } else if( HopRewriteUtils.isTransposeOperation(hop) ) { - out = tmp.get(hop.getInput().get(0).getHopID()); + out = tmp.get(hop.getInput().get(0).getHopID()); } else if( hop instanceof AggUnaryOp && ((AggUnaryOp)hop).getOp() == AggOp.SUM && ((AggUnaryOp)hop).getDirection() == Direction.RowCol ) http://git-wip-us.apache.org/repos/asf/systemml/blob/60ad522e/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java index 64014da..0389983 100644 --- a/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java +++ b/src/main/java/org/apache/sysml/hops/codegen/template/TemplateRow.java @@ -33,6 +33,7 @@ import org.apache.sysml.hops.LiteralOp; import org.apache.sysml.hops.ParameterizedBuiltinOp; import org.apache.sysml.hops.TernaryOp; import org.apache.sysml.hops.UnaryOp; +import org.apache.sysml.hops.codegen.SpoofCompiler; import org.apache.sysml.hops.codegen.cplan.CNode; import org.apache.sysml.hops.codegen.cplan.CNodeBinary; import org.apache.sysml.hops.codegen.cplan.CNodeBinary.BinType; @@ -83,7 +84,8 @@ public class TemplateRow extends TemplateBase && hop.getInput().get(0).getDim1()>1 && hop.getInput().get(0).getDim2()>1) || (hop instanceof AggBinaryOp && hop.dimsKnown() && LibMatrixMult.isSkinnyRightHandSide( hop.getInput().get(0).getDim1(), hop.getInput().get(0).getDim2(), //MM - hop.getInput().get(1).getDim1(), hop.getInput().get(1).getDim2()) + hop.getInput().get(1).getDim1(), hop.getInput().get(1).getDim2(), + SpoofCompiler.PLAN_SEL_POLICY.isCostBased()) && hop.getInput().get(0).getDim1()>1 && hop.getInput().get(0).getDim2()>1 && !HopRewriteUtils.isOuterProductLikeMM(hop)) || (HopRewriteUtils.isTransposeOperation(hop) && hop.getParent().size()==1 @@ -152,8 +154,9 @@ public class TemplateRow extends TemplateBase //check for fusable but not opening matrix multiply (vect_outer-mult) Hop in1 = hop.getInput().get(0); //transpose Hop in2 = hop.getInput().get(1); - return LibMatrixMult.isSkinnyRightHandSide(in1.getDim2(), in1.getDim1(), hop.getDim1(), hop.getDim2()) - || LibMatrixMult.isSkinnyRightHandSide(in2.getDim1(), in2.getDim2(), hop.getDim2(), hop.getDim1()); + boolean inclSizes = SpoofCompiler.PLAN_SEL_POLICY.isCostBased(); + return LibMatrixMult.isSkinnyRightHandSide(in1.getDim2(), in1.getDim1(), hop.getDim1(), hop.getDim2(), inclSizes) + || LibMatrixMult.isSkinnyRightHandSide(in2.getDim1(), in2.getDim2(), hop.getDim2(), hop.getDim1(), inclSizes); } private static boolean isPartOfValidCumAggChain(Hop hop) { http://git-wip-us.apache.org/repos/asf/systemml/blob/60ad522e/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java index eca26f6..fa4d667 100644 --- a/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java +++ b/src/main/java/org/apache/sysml/runtime/matrix/data/LibMatrixMult.java @@ -3567,13 +3567,13 @@ public class LibMatrixMult private static boolean checkPrepMatrixMultRightInput( MatrixBlock m1, MatrixBlock m2 ) { //transpose if dense-dense, skinny rhs matrix (not vector), and memory guarded by output return (LOW_LEVEL_OPTIMIZATION && !m1.sparse && !m2.sparse - && isSkinnyRightHandSide(m1.rlen, m1.clen, m2.rlen, m2.clen)); + && isSkinnyRightHandSide(m1.rlen, m1.clen, m2.rlen, m2.clen, true)); } //note: public for use by codegen for consistency - public static boolean isSkinnyRightHandSide(long m1rlen, long m1clen, long m2rlen, long m2clen) { + public static boolean isSkinnyRightHandSide(long m1rlen, long m1clen, long m2rlen, long m2clen, boolean inclCacheSize) { return m1rlen > m2clen && m2rlen > m2clen && m2clen > 1 - && m2clen < 64 && 8*m2rlen*m2clen < L2_CACHESIZE; + && m2clen < 64 && (!inclCacheSize || 8*m2rlen*m2clen < L2_CACHESIZE); } public static boolean checkParColumnAgg(MatrixBlock m1, int k, boolean inclFLOPs) {
