IntelLabs / RiverTrail

An API for data parallelism in JavaScript
Other
748 stars 71 forks source link

Error: non-scalar cast encountered #25

Closed hujiajie closed 11 years ago

hujiajie commented 11 years ago

hmm, maybe another bug:

Steps to reproduce:

var pa = new ParallelArray([1, 2], [3, 4]);
pa.map(function (x) {
  if (x[0] == 1) {}
  return x[0];
});

Expected result:

[1, 3]

Actual result:

Error: non-scalar cast encountered

And this time I have few ideas about the bug. Just some debugging results:

hujiajie commented 11 years ago

The error is thrown from this line. When the error is thrown, the variable ast, which is part of the AST representation of return x[0];, looks like this:

ast
 |-- value: "}" (I think this value is strange, is it right?)
 |-- type: 99 (I think it represents "CAST")
 |-- children
 |       |-- children[0]
 |               |-- value: "x"
 |               |-- rangeInfo: RangeArray
 |               |-- ...
 |-- ...
hujiajie commented 11 years ago

Then I tried to find out why there was a "CAST" here, though failed to understand the code. The AST seems all right until here, and the strange "CAST" node is pushed to the AST in RiverTrail.RangeAnalysis.propagate(ast, construct);. Or to be more specific, here's the call stack:

in https://github.com/RiverTrail/RiverTrail/blob/11d85730b72e977b18df27187ede4143a76c5b9b/jslib/jit/compiler/rangeanalysis.js#L1403 :

case THIS:
  ast.typeInfo = tEnv.lookup(ast.value).type.clone();
  // if the variable is a float but this expression is known to be
  // an int, we have to put a cast here
  if (isIntValue(ast) && (!validIntRepresentation(ast.typeInfo.OpenCLType))) {
    ast = makeCast(ast, "int");
    ast.rangeInfo = ast.children[0].rangeInfo; // inherit range info
  }
  break;

in https://github.com/RiverTrail/RiverTrail/blob/11d85730b72e977b18df27187ede4143a76c5b9b/jslib/jit/compiler/rangeanalysis.js#L1421

case INDEX:
  ast.children[1] = push(ast.children[1], tEnv, true);
  ast.children[0] = push(ast.children[0], tEnv, undefined);
  break;

in https://github.com/RiverTrail/RiverTrail/blob/11d85730b72e977b18df27187ede4143a76c5b9b/jslib/jit/compiler/rangeanalysis.js#L1259

case RETURN:
  ast.value = push(ast.value, tEnv, false); // we always return floating point values
  break;

in ...

hujiajie commented 11 years ago

isIntValue(ast) returns true here, is it right? By stepping into isIntValue, I came to here. I noticed that this._store was an empty array at that point, so this._store.every(...) returned true. Is that ok?

hujiajie commented 11 years ago

And further debugging. The following example works correctly:

var pa = new ParallelArray([1, 2], [3, 4]);
pa.map(function (x) {
  x[0] == 1;
  return x[0];
});

There's no "CAST" in the generated AST.

hujiajie commented 11 years ago

I also noticed that the rangeInfo of x was a Range object in the above sample, while in the previous discussion it was a RangeArray object. What does Range and RangeArray mean and what's the difference between them?

hujiajie commented 11 years ago

To find out why the rangeInfo is a 'RangeArray' object when if exists, I came to

https://github.com/RiverTrail/RiverTrail/blob/11d85730b72e977b18df27187ede4143a76c5b9b/jslib/jit/compiler/rangeanalysis.js#L355 :

VEp.apply = function (name, constraint) {
  var current = this.lookup(name);
  if (constraint instanceof Array) {
    if (current) {
      this.bindings[name] = new RangeArray(current, function (val,idx) { return val.constrain(constraint[idx]); });
    } else {
      this.bindings[name] = new RangeArray(constraint, function (val) { return new Range(val.lb, val.ub); });
    }
  } else {
    if (current) {
      this.bindings[name] = current.constrain(constraint);
    } else {
      this.bindings[name] = new Range(constraint.lb, constraint.ub);
    }
  }
  debug && console.log(name + " merged as " + this.bindings[name].toString());
};

in https://github.com/RiverTrail/RiverTrail/blob/11d85730b72e977b18df27187ede4143a76c5b9b/jslib/jit/compiler/rangeanalysis.js#L387 :

VEp.applyConstraints = function (constraints) {
  for (var name in constraints.bindings) {
    this.apply(name, constraints.lookup(name));
  }
}

in https://github.com/RiverTrail/RiverTrail/blob/11d85730b72e977b18df27187ede4143a76c5b9b/jslib/jit/compiler/rangeanalysis.js#L761 :

case IF:
  var predC = new Constraints();
  drive(ast.condition, varEnv, doAnnotate, predC);
  var thenVE = new VarEnv(varEnv);
  thenVE.applyConstraints(predC);
  drive(ast.thenPart, thenVE, doAnnotate);
  var predCE = new Constraints();
  drive(ast.condition, varEnv, doAnnotate, predCE, undefined, true); // compute inverse
  var elseVE = new VarEnv(varEnv);
  elseVE.applyConstraints(predCE);
  if (ast.elsePart) {
    drive(ast.elsePart, elseVE, doAnnotate);
  }
  thenVE.merge(elseVE);
  varEnv.updateAll(thenVE);
  break;

in ... in https://github.com/RiverTrail/RiverTrail/blob/11d85730b72e977b18df27187ede4143a76c5b9b/jslib/jit/compiler/driver.js#L176 :

function parse(paSource, construct, rankOrShape, kernel, args, lowPrecision) {
  var ast = RiverTrail.Helper.parseFunction(kernel);
  var rank = rankOrShape.length || rankOrShape;
  try {
    RiverTrail.Typeinference.analyze(ast, paSource, construct, rank, args, lowPrecision);
    RiverTrail.RangeAnalysis.analyze(ast, paSource, construct, rankOrShape, args);
    RiverTrail.RangeAnalysis.propagate(ast, construct);
    RiverTrail.InferBlockFlow.infer(ast);
    RiverTrail.InferMem.infer(ast);
  } catch (e) {
    RiverTrail.Helper.debugThrow(e);
  }
  return ast;
}

Then I got lost ...

hujiajie commented 11 years ago

Hope my report helps, if not misleads you. Thanks!

sherhut commented 11 years ago

Thank you for your report and in depth analysis. Your investigation made it a lot easier to hunt this one down. You actually found two bugs here.

1) The transition from undefined ranges to an array range was not implemented properly. That is what generated the RangeArray with an empty _store of ranges. 2) Constraints due to equalities were not handled correctly. The else branch (even if empty) should have produced a constraint that cancels out the one from the then branch. For ==, this was not the case, so the knowledge that x[0] is 1 escaped the conditional.

I have fixed both issues.

Thanks Stephan