Repository: systemml Updated Branches: refs/heads/master ea6dc8c58 -> 4e5ed3df6
[HOTFIX][SYSTEMML-2390] Fix pack IPA dead code elimination, JMLC tests This patch fix pack resolves multiple special cases of IPA dead code elimination with (1) Fix for external UDFs that are marked to have side-effects (2) Fix for correct memoization of processed hops (3) Fix for recursive partial operator removal from DAGs (4) Fix JMLC test for missing write for bound output variables (5) Cleanup constant folding rewrite (operator replacement) Project: http://git-wip-us.apache.org/repos/asf/systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/4e5ed3df Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/4e5ed3df Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/4e5ed3df Branch: refs/heads/master Commit: 4e5ed3df6bb853af393832b14cdef0fa3fc7c32e Parents: ea6dc8c Author: Matthias Boehm <[email protected]> Authored: Wed Jun 13 21:56:46 2018 -0700 Committer: Matthias Boehm <[email protected]> Committed: Wed Jun 13 23:27:08 2018 -0700 ---------------------------------------------------------------------- .../sysml/hops/ipa/FunctionCallGraph.java | 20 ++++++++++-- .../hops/ipa/IPAPassEliminateDeadCode.java | 15 +++++---- .../hops/rewrite/RewriteConstantFolding.java | 33 +++++--------------- .../sysml/utils/lite/BuildLiteExecution.java | 5 ++- 4 files changed, 38 insertions(+), 35 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/systemml/blob/4e5ed3df/src/main/java/org/apache/sysml/hops/ipa/FunctionCallGraph.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/ipa/FunctionCallGraph.java b/src/main/java/org/apache/sysml/hops/ipa/FunctionCallGraph.java index e889703..8db298e 100644 --- a/src/main/java/org/apache/sysml/hops/ipa/FunctionCallGraph.java +++ b/src/main/java/org/apache/sysml/hops/ipa/FunctionCallGraph.java @@ -171,6 +171,21 @@ public class FunctionCallGraph } /** + * Removes a single function call identified by target function name, + * and source function op and statement block. + * + * @param srcFkey source function key + * @param fkey function key of called function + * @param fop source function call operator + * @param sb source statement block + */ + public void removeFunctionCall(String srcFkey, String fkey, FunctionOp fop, StatementBlock sb) { + _fGraph.get(srcFkey).remove(fkey); + _fCalls.get(fkey).remove(fop); + _fCallsSB.get(fkey).remove(sb); + } + + /** * Indicates if the given function is either directly or indirectly recursive. * An example of an indirect recursive function is foo2 in the following call * chain: foo1 -> foo2 -> foo1. @@ -416,9 +431,8 @@ public class FunctionCallGraph private static boolean isSideEffectFree(FunctionStatementBlock fsb) { //check for side-effect-free external functions (explicit annotation) - if( fsb.getStatement(0) instanceof ExternalFunctionStatement - && !((ExternalFunctionStatement)fsb.getStatement(0)).hasSideEffects() ) { - return true; + if( fsb.getStatement(0) instanceof ExternalFunctionStatement ) { + return !((ExternalFunctionStatement)fsb.getStatement(0)).hasSideEffects(); } //check regular dml-bodied function for prints, pwrite, and other functions FunctionStatement fstmt = (FunctionStatement) fsb.getStatement(0); http://git-wip-us.apache.org/repos/asf/systemml/blob/4e5ed3df/src/main/java/org/apache/sysml/hops/ipa/IPAPassEliminateDeadCode.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/ipa/IPAPassEliminateDeadCode.java b/src/main/java/org/apache/sysml/hops/ipa/IPAPassEliminateDeadCode.java index 9ce79b0..344619e 100644 --- a/src/main/java/org/apache/sysml/hops/ipa/IPAPassEliminateDeadCode.java +++ b/src/main/java/org/apache/sysml/hops/ipa/IPAPassEliminateDeadCode.java @@ -98,12 +98,15 @@ public class IPAPassEliminateDeadCode extends IPAPass } private static void rRemoveOpFromDAG(Hop current) { - for( int i=0; i<current.getInput().size(); i++ ) { - Hop c = current.getInput().get(i); - HopRewriteUtils.removeChildReference(current, c); - if( c.getParent().isEmpty() ) - rRemoveOpFromDAG(c); + // cleanup child to parent links and + // recurse on operators ready for cleanup + for( Hop input : current.getInput() ) { + input.getParent().remove(current); + if( input.getParent().isEmpty() ) + rRemoveOpFromDAG(input); } + //cleanup parent to child links + current.getInput().clear(); } private static Set<String> rCollectReadVariableNames(StatementBlock sb, Set<String> varNames) { @@ -155,7 +158,7 @@ public class IPAPassEliminateDeadCode extends IPAPass rCollectReadVariableNames(c, varNames); if( HopRewriteUtils.isData(hop, DataOpTypes.TRANSIENTREAD) ) varNames.add(hop.getName()); - hop.isVisited(); + hop.setVisited(); return varNames; } } http://git-wip-us.apache.org/repos/asf/systemml/blob/4e5ed3df/src/main/java/org/apache/sysml/hops/rewrite/RewriteConstantFolding.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/rewrite/RewriteConstantFolding.java b/src/main/java/org/apache/sysml/hops/rewrite/RewriteConstantFolding.java index 0099029..e35d4cd 100644 --- a/src/main/java/org/apache/sysml/hops/rewrite/RewriteConstantFolding.java +++ b/src/main/java/org/apache/sysml/hops/rewrite/RewriteConstantFolding.java @@ -20,6 +20,7 @@ package org.apache.sysml.hops.rewrite; import java.util.ArrayList; +import java.util.List; import org.apache.sysml.conf.ConfigurationManager; import org.apache.sysml.hops.BinaryOp; @@ -115,36 +116,18 @@ public class RewriteConstantFolding extends HopRewriteRule } //replace binary operator with folded constant - if( literal != null ) - { - //reverse replacement in order to keep common subexpression elimination - int plen = root.getParent().size(); - if( plen > 0 ) //broot is NOT a DAG root - { - for( int i=0; i<root.getParent().size(); i++ ) //for all parents - { - Hop parent = root.getParent().get(i); - for( int j=0; j<parent.getInput().size(); j++ ) - { - Hop child = parent.getInput().get(j); - if( root == child ) - { - //replace operator - //root to parent link cannot be removed within this loop, as loop iterates over list containing parents. - parent.getInput().remove(j); - HopRewriteUtils.addChildReference(parent, literal,j); - } - } - } - root.getParent().clear(); + if( literal != null ) { + //bottom-up replacement to keep common subexpression elimination + if( !root.getParent().isEmpty() ) { //broot is NOT a DAG root + List<Hop> parents = new ArrayList<>(root.getParent()); + for( Hop parent : parents ) + HopRewriteUtils.replaceChildReference(parent, root, literal); } - else //broot IS a DAG root - { + else { //broot IS a DAG root root = literal; } } - //mark processed root.setVisited(); return root; http://git-wip-us.apache.org/repos/asf/systemml/blob/4e5ed3df/src/main/java/org/apache/sysml/utils/lite/BuildLiteExecution.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/utils/lite/BuildLiteExecution.java b/src/main/java/org/apache/sysml/utils/lite/BuildLiteExecution.java index 7b0d349..5f5d1f0 100644 --- a/src/main/java/org/apache/sysml/utils/lite/BuildLiteExecution.java +++ b/src/main/java/org/apache/sysml/utils/lite/BuildLiteExecution.java @@ -404,7 +404,10 @@ public class BuildLiteExecution log.debug(displayMatrix(cgBetas)); String glmPredict = conn.readScript("scripts/algorithms/GLM-predict.dml"); - PreparedScript glmPredictScript = conn.prepareScript(glmPredict, new String[] { "X", "Y", "B_full" }, + HashMap<String,String> args = new HashMap<>(); + args.put("$M", "/tmp/dummy"); //required due to conditional write + PreparedScript glmPredictScript = conn.prepareScript( + glmPredict, args, new String[] { "X", "Y", "B_full" }, new String[] { "means" }, false); double[][] testData = new double[500][3]; for (int i = 0; i < 500; i++) {
