Repository: systemml Updated Branches: refs/heads/master 60d20c2b7 -> f991e4b48
[SYSTEMML-1918] Fix rewrite 'merge block sequence' (tmp invalid DAGs) This patch fixes the statement block rewrite RewriteMergeBlockSequence, which caused temporarily invalid HOP DAGs (e.g., twrites without inputs) in the original program because the list of statement blocks is modified and only finally attached back to the statement block hierarchy. This creates problems for other statement block rewrites, which need to make a full pass over the program for global analysis. Furthermore, this patch also eliminates a debug condition, which unnecessarily disabled this rewrite for special cases. Project: http://git-wip-us.apache.org/repos/asf/systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/945ca7b4 Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/945ca7b4 Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/945ca7b4 Branch: refs/heads/master Commit: 945ca7b4b937b2e6a391f94bf9d45d34637022ac Parents: 60d20c2 Author: Matthias Boehm <mboe...@gmail.com> Authored: Sat Sep 16 02:11:10 2017 -0700 Committer: Matthias Boehm <mboe...@gmail.com> Committed: Sat Sep 16 17:21:45 2017 -0700 ---------------------------------------------------------------------- .../sysml/hops/rewrite/HopRewriteUtils.java | 12 +++++++++++ .../hops/rewrite/RewriteMergeBlockSequence.java | 22 +++++++------------- 2 files changed, 19 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/systemml/blob/945ca7b4/src/main/java/org/apache/sysml/hops/rewrite/HopRewriteUtils.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/rewrite/HopRewriteUtils.java b/src/main/java/org/apache/sysml/hops/rewrite/HopRewriteUtils.java index 453bbd9..7093b0e 100644 --- a/src/main/java/org/apache/sysml/hops/rewrite/HopRewriteUtils.java +++ b/src/main/java/org/apache/sysml/hops/rewrite/HopRewriteUtils.java @@ -55,7 +55,12 @@ import org.apache.sysml.hops.UnaryOp; import org.apache.sysml.hops.Hop.OpOp1; import org.apache.sysml.parser.DataExpression; import org.apache.sysml.parser.DataIdentifier; +import org.apache.sysml.parser.ForStatementBlock; +import org.apache.sysml.parser.FunctionStatementBlock; +import org.apache.sysml.parser.IfStatementBlock; import org.apache.sysml.parser.Statement; +import org.apache.sysml.parser.StatementBlock; +import org.apache.sysml.parser.WhileStatementBlock; import org.apache.sysml.parser.Expression.DataType; import org.apache.sysml.parser.Expression.ValueType; import org.apache.sysml.runtime.instructions.cp.ScalarObject; @@ -1159,4 +1164,11 @@ public class HopRewriteUtils long size2 = hop2.getDim1() * hop2.getDim2(); return Long.compare(size1, size2); } + + public static boolean isLastLevelStatementBlock(StatementBlock sb) { + return !(sb instanceof FunctionStatementBlock + || sb instanceof WhileStatementBlock + || sb instanceof IfStatementBlock + || sb instanceof ForStatementBlock); //incl parfor + } } http://git-wip-us.apache.org/repos/asf/systemml/blob/945ca7b4/src/main/java/org/apache/sysml/hops/rewrite/RewriteMergeBlockSequence.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/rewrite/RewriteMergeBlockSequence.java b/src/main/java/org/apache/sysml/hops/rewrite/RewriteMergeBlockSequence.java index 746e536..659ad2e 100644 --- a/src/main/java/org/apache/sysml/hops/rewrite/RewriteMergeBlockSequence.java +++ b/src/main/java/org/apache/sysml/hops/rewrite/RewriteMergeBlockSequence.java @@ -28,12 +28,8 @@ import org.apache.sysml.hops.FunctionOp; import org.apache.sysml.hops.Hop; import org.apache.sysml.hops.Hop.DataOpTypes; import org.apache.sysml.hops.HopsException; -import org.apache.sysml.parser.ForStatementBlock; -import org.apache.sysml.parser.FunctionStatementBlock; -import org.apache.sysml.parser.IfStatementBlock; import org.apache.sysml.parser.StatementBlock; import org.apache.sysml.parser.VariableSet; -import org.apache.sysml.parser.WhileStatementBlock; /** * Rule: Simplify program structure by merging sequences of last-level @@ -66,9 +62,9 @@ public class RewriteMergeBlockSequence extends StatementBlockRewriteRule for( int i=0; i<tmpList.size()-1; i++ ) { StatementBlock sb1 = tmpList.get(i); StatementBlock sb2 = tmpList.get(i+1); - if( isLastLevelStatementBlock(sb1) && isLastLevelStatementBlock(sb2) - && !hasFunctionOpRoot(sb1) && !sb1.isSplitDag() && !sb2.isSplitDag() - && sb2.getBeginLine()!=34 ) + if( HopRewriteUtils.isLastLevelStatementBlock(sb1) + && HopRewriteUtils.isLastLevelStatementBlock(sb2) + && !hasFunctionOpRoot(sb1) && !sb1.isSplitDag() && !sb2.isSplitDag() ) { ArrayList<Hop> sb1Hops = sb1.get_hops(); ArrayList<Hop> sb2Hops = sb2.get_hops(); @@ -106,9 +102,12 @@ public class RewriteMergeBlockSequence extends StatementBlockRewriteRule sb2Hops.add(root); } } - Hop.resetVisitStatus(sb2Hops); + //clear partial hops from the merged statement block to avoid problems with + //other statement block rewrites that iterate over the original program + sb1Hops.clear(); //run common-subexpression elimination + Hop.resetVisitStatus(sb2Hops); rewriter.rewriteHopDAGs(sb2Hops, new ProgramRewriteStatus()); //modify live variable sets of s2 @@ -164,11 +163,4 @@ public class RewriteMergeBlockSequence extends StatementBlockRewriteRule ret |= (root instanceof FunctionOp); return ret; } - - private static boolean isLastLevelStatementBlock(StatementBlock sb) { - return !((sb instanceof FunctionStatementBlock) - || (sb instanceof WhileStatementBlock) - || (sb instanceof IfStatementBlock) - || (sb instanceof ForStatementBlock)); //incl parfor - } }
