From 19c307d403be19a7c9d620e475ade07523d1caa8 Mon Sep 17 00:00:00 2001
From: stepan <stepan.sindelar@oracle.com>
Date: Thu, 29 Sep 2016 16:05:27 +0200
Subject: [PATCH] rbdiag supports two levels of matching the outputs produced
by FastR and GnuR
---
.../r/nodes/casts/FilterSamplerFactory.java | 4 +-
.../truffle/r/nodes/test/ChimneySweeping.java | 62 +++++++++++++++++--
.../r/nodes/test/DefaultArgsExtractor.java | 11 +++-
.../r/nodes/test/PipelineToCastNodeTests.java | 10 +++
.../builtin/casts/PipelineToCastNode.java | 33 ++++++++--
mx.fastr/mx_fastr.py | 10 +--
6 files changed, 109 insertions(+), 21 deletions(-)
diff --git a/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/casts/FilterSamplerFactory.java b/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/casts/FilterSamplerFactory.java
index f053c1a13f..cac9bd6aa3 100644
--- a/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/casts/FilterSamplerFactory.java
+++ b/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/casts/FilterSamplerFactory.java
@@ -174,7 +174,7 @@ public final class FilterSamplerFactory
return leftFilter.trueBranchType().or(rightFilter.trueBranchType());
}
- @SuppressWarnings({"unchecked", "cast"})
+ @SuppressWarnings("unchecked")
@Override
public Samples<Object> collectSamples(TypeExpr inputType) {
Samples thisSamples = leftFilter.collectSamples(inputType);
@@ -204,7 +204,7 @@ public final class FilterSamplerFactory
@Override
public TypeExpr trueBranchType() {
- return filter.getFilter().isNarrowing() ? trueBranchType().not() : trueBranchType();
+ return filter.getFilter().isNarrowing() ? toNegate.trueBranchType().not() : toNegate.trueBranchType();
}
@SuppressWarnings({"unchecked", "cast"})
diff --git a/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/ChimneySweeping.java b/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/ChimneySweeping.java
index f82773e5bb..a02b86e9fc 100644
--- a/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/ChimneySweeping.java
+++ b/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/ChimneySweeping.java
@@ -69,9 +69,11 @@ class ChimneySweeping extends SingleBuiltinDiagnostics {
private static final String TEST_PREFIX = "com.oracle.truffle.r.test.builtins.TestBuiltin_";
private static final String SWEEP_MODE_ARG = "--sweep";
- private static final String SWEEP_MODE_ARG_SPEC = SWEEP_MODE_ARG + "-";
+ private static final String SWEEP_MODE_ARG_SPEC = SWEEP_MODE_ARG + "=";
private static final String NO_SELF_TEST_ARG = "--noSelfTest";
private static final String MISSING_AND_NULL_SAMPLES_ONLY_ARG = "--mnonly";
+ private static final String OUTPUT_MATCH_LEVEL = "--matchLevel";
+ private static final String OUTPUT_MATCH_LEVEL_SPEC = OUTPUT_MATCH_LEVEL + "=";
enum ChimneySweepingMode {
auto,
@@ -89,8 +91,22 @@ class ChimneySweeping extends SingleBuiltinDiagnostics {
}
}
+ enum OutputMatchLevel {
+ same,
+ error;
+
+ static Optional<OutputMatchLevel> fromArg(String arg) {
+ if (arg.startsWith(OUTPUT_MATCH_LEVEL_SPEC)) {
+ return Optional.of(valueOf(arg.substring(OUTPUT_MATCH_LEVEL_SPEC.length())));
+ } else {
+ return Optional.empty();
+ }
+ }
+ }
+
static class ChimneySweepingConfig extends DiagConfig {
ChimneySweepingMode sweepingMode;
+ OutputMatchLevel outputMatchLevel;
boolean missingAndNullSamplesOnly;
boolean performPipelineSelfTest;
}
@@ -127,6 +143,7 @@ class ChimneySweeping extends SingleBuiltinDiagnostics {
static <C extends ChimneySweepingConfig> C initChimneySweepingConfig(C config, String[] args) {
config.sweepingMode = getSweepMode(args).flatMap(ChimneySweepingMode::fromArg).orElse(ChimneySweepingMode.auto);
+ config.outputMatchLevel = getOutputMatchLevel(args).flatMap(OutputMatchLevel::fromArg).orElse(OutputMatchLevel.same);
config.missingAndNullSamplesOnly = Arrays.stream(args).filter(arg -> MISSING_AND_NULL_SAMPLES_ONLY_ARG.equals(arg)).findFirst().isPresent();
// The pipeline self-test is disabled when only RMissing and RNull samples are used as
// these values are not determined via the pipeline static type analysis
@@ -138,6 +155,10 @@ class ChimneySweeping extends SingleBuiltinDiagnostics {
return Arrays.stream(args).filter(arg -> arg.startsWith(SWEEP_MODE_ARG)).findFirst();
}
+ private static Optional<String> getOutputMatchLevel(String[] args) {
+ return Arrays.stream(args).filter(arg -> arg.startsWith(OUTPUT_MATCH_LEVEL_SPEC)).findFirst();
+ }
+
@Override
public SingleBuiltinDiagnostics createBuiltinDiagnostics(RBuiltinDiagFactory bf) {
if (bf instanceof RIntBuiltinDiagFactory) {
@@ -331,9 +352,14 @@ class ChimneySweeping extends SingleBuiltinDiagnostics {
}
private static RList evalValidArgs(String argsExpr, PolyglotEngine vm) {
- Value eval = vm.eval(RSource.fromTextInternal(argsExpr, RSource.Internal.UNIT_TEST));
- RList args = (RList) eval.get();
- return args;
+ try {
+ Value eval = vm.eval(RSource.fromTextInternal(argsExpr, RSource.Internal.UNIT_TEST));
+ RList args = (RList) eval.get();
+ return args;
+ } catch (Exception e) {
+ System.out.println("Warning: Cannot parse arguments: " + argsExpr);
+ return null;
+ }
}
private void sweepChimney() throws IOException {
@@ -394,6 +420,9 @@ class ChimneySweeping extends SingleBuiltinDiagnostics {
try {
for (int i = 0; i < args.size(); i++) {
Object validArg = args.get(i);
+ if (validArg == RMissing.instance) {
+ continue;
+ }
String deparsedValidArg;
try {
deparsedValidArg = RDeparse.deparse(validArg);
@@ -404,7 +433,7 @@ class ChimneySweeping extends SingleBuiltinDiagnostics {
if (sb.length() != 0) {
sb.append(",");
}
- sb.append(deparsedValidArg);
+ sb.append(parameterNames[i]).append('=').append(deparsedValidArg);
}
String call;
@@ -427,7 +456,7 @@ class ChimneySweeping extends SingleBuiltinDiagnostics {
List<String> outputPair = Arrays.asList(output, outputGnu);
- if (outputGnu.equals(output)) {
+ if (compareOutputs(output, outputGnu)) {
System.out.print('.');
} else if (!printedOutputPairs.contains(outputPair)) {
System.out.println("\n#" + sweepCounter + "> " + call);
@@ -457,6 +486,27 @@ class ChimneySweeping extends SingleBuiltinDiagnostics {
}
}
+ private boolean compareOutputs(String output, String outputGnu) {
+ switch (diagSuite.diagConfig.outputMatchLevel) {
+ case same:
+ return outputGnu.equals(output);
+ case error:
+ if (output.contains("ERROR:")) {
+ // FastR error
+ return false;
+ }
+ if (output.contains("Error") && outputGnu.contains("Error")) {
+ return true;
+ }
+ if (!output.contains("Error") && !outputGnu.contains("Error")) {
+ return true;
+ }
+ return false;
+ default:
+ throw new UnsupportedOperationException("Unsupported output match level: " + diagSuite.diagConfig.outputMatchLevel);
+ }
+ }
+
static long calculateNumOfSampleCombinations(List<Samples<?>> argSamples) {
Long total = argSamples.stream().reduce(1L, (num, samples) -> num * (samples.allSamples().size() + 1), (n1, n2) -> n1 * n2);
return total;
diff --git a/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/DefaultArgsExtractor.java b/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/DefaultArgsExtractor.java
index 6a93d4f48a..c11be9eb79 100644
--- a/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/DefaultArgsExtractor.java
+++ b/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/DefaultArgsExtractor.java
@@ -71,9 +71,14 @@ class DefaultArgsExtractor {
if (defVal instanceof RLanguage) {
String deparsedDefVal = RDeparse.deparse(defVal);
- Value eval = vm.eval(RSource.fromTextInternal(deparsedDefVal,
- RSource.Internal.UNIT_TEST));
- defVal = eval.get();
+ try {
+ Value eval = vm.eval(RSource.fromTextInternal(deparsedDefVal,
+ RSource.Internal.UNIT_TEST));
+ defVal = eval.get();
+ } catch (Exception e) {
+ System.out.println("Warning: Unable to evaluate the default value of argument " + name + ". Expression: " + deparsedDefVal);
+ continue;
+ }
if (defVal == null) {
samplesMap.put(name, Samples.anything(RNull.instance));
diff --git a/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/PipelineToCastNodeTests.java b/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/PipelineToCastNodeTests.java
index 8b6719220b..e31dde44de 100644
--- a/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/PipelineToCastNodeTests.java
+++ b/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/PipelineToCastNodeTests.java
@@ -27,6 +27,7 @@ import static org.junit.Assert.assertEquals;
import org.junit.Test;
+import com.oracle.truffle.r.nodes.builtin.casts.Filter.OrFilter;
import com.oracle.truffle.r.nodes.builtin.casts.Filter.RTypeFilter;
import com.oracle.truffle.r.nodes.builtin.casts.Filter.TypeFilter;
import com.oracle.truffle.r.nodes.builtin.casts.Mapper.MapByteToBoolean;
@@ -79,6 +80,15 @@ public class PipelineToCastNodeTests {
assertBypassNode(pipeline, BypassIntegerNode.class);
}
+ @Test
+ public void optimizeForSingleIntegerWhenNumericFilterIsUsed() {
+ // should be close to mustBe(numericValue()).asIntegerVector().findFirst()
+ OrFilter<Object> filter = new OrFilter<>(new RTypeFilter<>(RType.Integer), new RTypeFilter<>(RType.Double));
+ CastNode pipeline = createPipeline(
+ new FilterStep<>(filter, null, false).setNext(new CoercionStep<>(RType.Integer, false).setNext(new FindFirstStep<>(null, Integer.class, null))));
+ assertBypassNode(pipeline, BypassIntegerNode.class);
+ }
+
@Test
public void optimizeMapSingleByteToBoolean() {
// should be equivalent of mustBe(integerValue()).asIntegerVector().findFirst()
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/builtin/casts/PipelineToCastNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/builtin/casts/PipelineToCastNode.java
index 2f7182b34f..48823ac036 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/builtin/casts/PipelineToCastNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/builtin/casts/PipelineToCastNode.java
@@ -187,7 +187,7 @@ public final class PipelineToCastNode {
@Override
public CastNode visit(FindFirstStep<?, ?> step) {
- assert targetType != null : "There must be a coercion step before find first";
+ assert !canBeOptimized || targetType != null : "There must be a coercion step before find first";
findFirstStep = step;
return inner.visit(step);
}
@@ -212,11 +212,9 @@ public final class PipelineToCastNode {
@Override
public CastNode visit(FilterStep<?, ?> step) {
- Filter<?, ?> filter = step.getFilter();
- if (filter instanceof RTypeFilter) {
- canBeOptimized(((RTypeFilter<?>) filter).getType());
- } else {
- cannotBeOptimizedBeforeFindFirst();
+ targetType = checkFilter(step.getFilter());
+ if (targetType == null) {
+ canBeOptimized = false;
}
return inner.visit(step);
}
@@ -270,6 +268,29 @@ public final class PipelineToCastNode {
}
}
+ /**
+ * Returns null if the filter does not conform to expected type or does not produce some
+ * concrete type if there is no expected type.
+ */
+ private RType checkFilter(Filter<?, ?> filter) {
+ if (filter instanceof RTypeFilter) {
+ RType type = ((RTypeFilter) filter).getType();
+ if (targetType == null) {
+ return type;
+ }
+ return type == targetType ? type : null;
+ } else if (filter instanceof OrFilter) {
+ OrFilter<?> or = (OrFilter<?>) filter;
+ RType leftType = checkFilter(or.getLeft());
+ if (targetType == null) {
+ return leftType;
+ }
+ RType rightType = checkFilter(or.getRight());
+ return rightType == targetType || leftType == targetType ? targetType : null;
+ }
+ return null;
+ }
+
private CastNode getFindFirstWithDefault() {
if (findFirstStep != null && findFirstStep.getDefaultValue() != null) {
return convert(findFirstStep, inner);
diff --git a/mx.fastr/mx_fastr.py b/mx.fastr/mx_fastr.py
index d78a9e55b4..2b63d8eecd 100644
--- a/mx.fastr/mx_fastr.py
+++ b/mx.fastr/mx_fastr.py
@@ -462,9 +462,11 @@ def rbdiag(args):
--mnonly Uses the RMissing and RNull values as the only samples for the chimney-sweeping
--noSelfTest Does not perform the pipeline self-test using the generated samples as the intro to each chimney-sweeping. It has no effect when --mnonly is specified as the self-test is never performed in that case.
--sweep Performs the 'chimney-sweeping'. The sample combination selection method is determined automatically.
- --sweep-lite Performs the 'chimney-sweeping'. The diagonal sample selection method is used.
- --sweep-total Performs the 'chimney-sweeping'. The total sample selection method is used.
-
+ --sweep=lite Performs the 'chimney-sweeping'. The diagonal sample selection method is used.
+ --sweep=total Performs the 'chimney-sweeping'. The total sample selection method is used.
+ --matchLevel=same Outputs produced by FastR and GnuR must be same (default)
+ --matchLevel=error Outputs are considered matching if none or both outputs contain an error
+
If no builtin is specified, all registered builtins are diagnosed.
An external builtin is specified by the fully qualified name of its node class.
@@ -527,7 +529,7 @@ _commands = {
'junitnopkgs' : [junit_nopkgs, ['options']],
'unittest' : [unittest, ['options']],
'rbcheck' : [rbcheck, '--filter [gnur-only,fastr-only,both,both-diff]'],
- 'rbdiag' : [rbdiag, '(builtin)* [-v] [-n] [-m] [--sweep | --sweep-lite | --sweep-total] [--mnonly] [--noSelfTest]'],
+ 'rbdiag' : [rbdiag, '(builtin)* [-v] [-n] [-m] [--sweep | --sweep=lite | --sweep=total] [--mnonly] [--noSelfTest] [--matchLevel=same | --matchLevel=error]'],
'rcmplib' : [rcmplib, ['options']],
'rrepl' : [rrepl, '[options]'],
'rembed' : [rembed, '[options]'],
--
GitLab