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 f053c1a13f5f38b2f95d4c34476ae0076b57be2d..cac9bd6aa33d4975a36064290e6de9782f89a520 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 f82773e5bb13a551042fa322a099d11ad766f2af..a02b86e9fcf47a235260ee347854e88b31056232 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 6a93d4f48abf3ec84d2d70bfe370a71285f40351..c11be9eb7954ccb7c3b01d65174081ab484896cf 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 8b6719220bd1d956ee48632818ddd5878e9da1e1..e31dde44de60c70dff2dec4c18d88e74eabb0a4a 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 2f7182b34f42797448707d4f83fe8953ba2f8225..48823ac0367731235da1e250f12a26b0e0a80c2a 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 d78a9e55b41acb1a133c43a17ef3c41bf61e30fc..2b63d8eecd533526d5ec7ef971e50e033f73991c 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]'],