[SYSTEMML-2429] Fix IPA function inlining pass w/ partial binding This patch fixes a side effect between IPA function inlining and IPA dead code elimination, where the dead code elimination failed due to a function call graph corruption on partial inlining. Specifically such a partial inlining happens when a subset of function calls does not bind all output parameters in which case these calls are not inlined.
Project: http://git-wip-us.apache.org/repos/asf/systemml/repo Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/a1eb7bce Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/a1eb7bce Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/a1eb7bce Branch: refs/heads/master Commit: a1eb7bcedef100ef66f35eada2054fcbff00f6bc Parents: e83ae65 Author: Matthias Boehm <[email protected]> Authored: Sun Jul 8 14:05:23 2018 -0700 Committer: Matthias Boehm <[email protected]> Committed: Sun Jul 8 14:27:36 2018 -0700 ---------------------------------------------------------------------- .../sysml/hops/ipa/IPAPassInlineFunctions.java | 8 ++- .../sysml/hops/ipa/InterProceduralAnalysis.java | 2 +- .../functions/misc/FunctionPotpourriTest.java | 8 ++- .../misc/FunPotpourriSubsetReturnDead.dml | 51 ++++++++++++++++++++ 4 files changed, 65 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/systemml/blob/a1eb7bce/src/main/java/org/apache/sysml/hops/ipa/IPAPassInlineFunctions.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/ipa/IPAPassInlineFunctions.java b/src/main/java/org/apache/sysml/hops/ipa/IPAPassInlineFunctions.java index 7706ed6..4c7998d 100644 --- a/src/main/java/org/apache/sysml/hops/ipa/IPAPassInlineFunctions.java +++ b/src/main/java/org/apache/sysml/hops/ipa/IPAPassInlineFunctions.java @@ -72,6 +72,7 @@ public class IPAPassInlineFunctions extends IPAPass ArrayList<Hop> hops = fstmt.getBody().get(0).getHops(); List<FunctionOp> fcalls = fgraph.getFunctionCalls(fkey); List<StatementBlock> fcallsSB = fgraph.getFunctionCallsSB(fkey); + boolean removedAll = true; for(int i=0; i<fcalls.size(); i++) { FunctionOp op = fcalls.get(i); if( LOG.isDebugEnabled() ) @@ -79,8 +80,10 @@ public class IPAPassInlineFunctions extends IPAPass //step 0: robustness for special cases if( op.getInput().size() != fstmt.getInputParams().size() - || op.getOutputVariableNames().length != fstmt.getOutputParams().size() ) + || op.getOutputVariableNames().length != fstmt.getOutputParams().size() ) { + removedAll = false; continue; + } //step 1: deep copy hop dag ArrayList<Hop> hops2 = Recompiler.deepCopyHopsDag(hops); @@ -110,7 +113,8 @@ public class IPAPassInlineFunctions extends IPAPass //update the function call graph to avoid repeated inlining //(and thus op replication) on repeated IPA calls - fgraph.removeFunctionCalls(fkey); + if( removedAll ) + fgraph.removeFunctionCalls(fkey); } } } http://git-wip-us.apache.org/repos/asf/systemml/blob/a1eb7bce/src/main/java/org/apache/sysml/hops/ipa/InterProceduralAnalysis.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/sysml/hops/ipa/InterProceduralAnalysis.java b/src/main/java/org/apache/sysml/hops/ipa/InterProceduralAnalysis.java index c00ea6c..85a433a 100644 --- a/src/main/java/org/apache/sysml/hops/ipa/InterProceduralAnalysis.java +++ b/src/main/java/org/apache/sysml/hops/ipa/InterProceduralAnalysis.java @@ -101,7 +101,7 @@ public class InterProceduralAnalysis static { // for internal debugging only if( LDEBUG ) { - Logger.getLogger("org.apache.sysml.hops.ipa.InterProceduralAnalysis") + Logger.getLogger("org.apache.sysml.hops.ipa") .setLevel((Level) Level.DEBUG); } } http://git-wip-us.apache.org/repos/asf/systemml/blob/a1eb7bce/src/test/java/org/apache/sysml/test/integration/functions/misc/FunctionPotpourriTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/sysml/test/integration/functions/misc/FunctionPotpourriTest.java b/src/test/java/org/apache/sysml/test/integration/functions/misc/FunctionPotpourriTest.java index e7634fa..fda4c2f 100644 --- a/src/test/java/org/apache/sysml/test/integration/functions/misc/FunctionPotpourriTest.java +++ b/src/test/java/org/apache/sysml/test/integration/functions/misc/FunctionPotpourriTest.java @@ -33,7 +33,7 @@ public class FunctionPotpourriTest extends AutomatedTestBase private final static String TEST_NAME3 = "FunPotpourriNoReturn2"; private final static String TEST_NAME4 = "FunPotpourriEval"; private final static String TEST_NAME5 = "FunPotpourriSubsetReturn"; - + private final static String TEST_NAME6 = "FunPotpourriSubsetReturnDead"; private final static String TEST_DIR = "functions/misc/"; private final static String TEST_CLASS_DIR = TEST_DIR + FunctionPotpourriTest.class.getSimpleName() + "/"; @@ -46,6 +46,7 @@ public class FunctionPotpourriTest extends AutomatedTestBase addTestConfiguration( TEST_NAME3, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME3, new String[] { "R" }) ); addTestConfiguration( TEST_NAME4, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME4, new String[] { "R" }) ); addTestConfiguration( TEST_NAME5, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME5, new String[] { "R" }) ); + addTestConfiguration( TEST_NAME6, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME6, new String[] { "R" }) ); } @Test @@ -73,6 +74,11 @@ public class FunctionPotpourriTest extends AutomatedTestBase runFunctionTest( TEST_NAME5, false ); } + @Test + public void testFunctionSubsetReturnDead() { + runFunctionTest( TEST_NAME6, false ); + } + private void runFunctionTest(String testName, boolean error) { TestConfiguration config = getTestConfiguration(testName); loadTestConfiguration(config); http://git-wip-us.apache.org/repos/asf/systemml/blob/a1eb7bce/src/test/scripts/functions/misc/FunPotpourriSubsetReturnDead.dml ---------------------------------------------------------------------- diff --git a/src/test/scripts/functions/misc/FunPotpourriSubsetReturnDead.dml b/src/test/scripts/functions/misc/FunPotpourriSubsetReturnDead.dml new file mode 100644 index 0000000..c7ed6fd --- /dev/null +++ b/src/test/scripts/functions/misc/FunPotpourriSubsetReturnDead.dml @@ -0,0 +1,51 @@ +#------------------------------------------------------------- +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +#------------------------------------------------------------- + +arima_residuals = function(Matrix[Double] weights, Matrix[Double] X, Integer p, Integer P, Integer q, Integer Q, Integer s, String solver) return (Matrix[Double] errs, Matrix[Double] combined_weights){ + combined_weights = weights + if (p>0 & P>0) + combined_weights = rbind(combined_weights, matrix(weights[1:p,] %*% t(weights[p+1:p+P,]), rows=p*P, cols=1)) + b = X[,2:ncol(X)]%*%combined_weights + errs = X[,1] - b +} + +X = matrix(1, 1000, 1) +p = 2 +d = 0 +q = 0 +P = 0 +D = 0 +Q = 0 +s = 0 +totparamcols = p+P+Q+q+p*P +num_rows = nrow(X) + +if(num_rows <= d) + print("non-seasonal differencing order should be smaller than length of the time-series") +if(num_rows <= s*D) + print("seasonal differencing order should be smaller than number of observations divided by length of season") + +Z = cbind (X[1:nrow(X),], matrix(0, nrow(X), totparamcols)) +weights = matrix("0.459982 0.673987", 2, 1) + +f1 = arima_residuals(weights, Z, p, P, q, Q, s, "") +f2 = arima_residuals(weights, Z, p, P, q, Q, s, "") +print("out: " + sum(f1))
