You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@systemml.apache.org by mb...@apache.org on 2018/06/14 06:26:54 UTC

systemml git commit: [HOTFIX][SYSTEMML-2390] Fix pack IPA dead code elimination, JMLC tests

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 <mb...@gmail.com>
Authored: Wed Jun 13 21:56:46 2018 -0700
Committer: Matthias Boehm <mb...@gmail.com>
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 -&gt; foo2 -&gt; 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++) {