chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.78k stars 420 forks source link

Complicated Ternary Expression Bug #24275

Closed damianmoz closed 8 months ago

damianmoz commented 8 months ago

$ chpl --fast --ieee-float c.chpl
c.chpl:240: internal error: RES-RES-ION-2902 chpl version 1.33.0 Note: This source location is a guess.

Internal errors indicate a bug in the Chapel compiler, and we're sorry for the hassle. We would appreciate your reporting this bug -- please see https://chapel-lang.org/bugs.html for instructions. In the meantime, the filename + line number above may be useful in working around the issue. bug.txt

Sorry for the typo at my first attempt.

mppf commented 8 months ago

@damianmoz - did you mean to include some source code to demonstrate the problem? I'm not seeing such a thing here.

damianmoz commented 8 months ago

Sorry. See: bug.txt

jabraham17 commented 8 months ago

Posting a cut-down version of a reproducer


config const n = 48020;
config const Y = 1;

proc vmax(const ref x : [] real(?w), const ref y : [] real(w))
{
  var t = 0:real(w);

  foreach (tx, ty) in zip(x, y) do t = max(t, abs(tx * ty));

  return t;
}

proc maxOverColumnsTailAtEnd(a : [?D] real(?w), r : [] real(w))
{
    const (R, C) = D.dims();
    const B = 0 ..< min(R.size, Y*here.maxTaskPar);

    var _c : [B, C] real(w);
    return [j in C] max reduce _c[.., j];
}

config param w = 64;

proc check(mask : int)
{
    const W = 0..<n;
    var a : [0..<n, 0..<n] real(w);
    var r : [0..<n] real(w);

  const alg = 1;

  var c = if alg == 0 then
    for j in 0..<n do max reduce abs(a[.., j] * r[..])
  else if alg == 1 then
    [ j in 0..<n ] vmax(a[.., j], r[..])
  else
    maxOverColumnsTailAtEnd(a, r);
}

config const mask = 1 | 2 | 4 | 8 | 16;

proc main
{
    check(mask);
    return(0);
}

Compiling this gives the following output

bug.chpl:35: internal error: not handled [resolution/resolveFunction.cpp:2906]
Note: This source location is a guess.
cassella commented 8 months ago

Slightly simpler reproducer,

config const n = 48020;

proc maxOverColumnsTailAtEnd(a : [?D] real, r : [] real)
{
  return for j in 0..<n do max reduce abs(a[.., j] * r[..]);
}

proc check()
{
    var a : [0..<n, 0..<n] real;
    var r : [0..<n] real;

  const alg = 1;

  var c = if alg == 0 then
    for j in 0..<n do max reduce abs(a[.., j] * r[..])
  else
    maxOverColumnsTailAtEnd(a, r);
}

proc main
{
    check();
    return(0);
}

(Note I replaced the body of maxOverColumnsTailAtEnd() with the same for expression as in the alg==0 case.)

The error is in insertInitConversion() which seems to have concluded it's operating on a sync or single. There are no syncs or singles in the reproducer.

  if (!typesDiffer && !useRttCopy) {
    // types are the same and no runtime types.
    ...
    return;
  }
  if (useRttCopy) {
...
  } else {
...
    if (toType->type->symbol->hasFlag(FLAG_TUPLE)) {
    } else if (toType->type == getCopyTypeDuringResolution(fromValType)) {
      // today, this code should only apply to sync/single
      // (since arrays are handled above with useRttCopy)
      // for sync/single, getCopyTypeDuringResolution returns the valType.
...
      if (isSyncType(fromValType)) { ...
      } else if (isSingleType(fromValType)) { ...
      } else {
        INT_FATAL("not handled");

I wondered if this could be worked around with a split init, but it runs into #24108:

  var c;
  if alg == 0 then
    c = for j in 0..<n do max reduce abs(a[.., j] * r[..]);
  else if alg == 1 then
    c = [ j in 0..<n ] vmax(a[.., j], r[..]);
  else
    c = maxOverColumnsTailAtEnd(a, r);

Here's a bonus,

  var c = if alg == 0 then
    for j in 0..<n do max reduce abs(a[.., j] * r[..])
  else
    for j in 0..<n do max reduce abs(a[.., j] * r[..]);
foo.chpl:10: In function 'check':
foo.chpl:19: error: Unable to resolve type of if-expression
note: 'then' branch returns type "_ir_chpl__loopexpr_iter12"
note: 'else' branch returns type "_ir_chpl__loopexpr_iter13"

Here's a workaround for Jade's cut-down example, pulling out the expressions in the ternary into their own functions:

config const n = 48020;
config const Y = 1;

proc vmax(const ref x : [] real(?w), const ref y : [] real(w))
{
  var t = 0:real(w);

  foreach (tx, ty) in zip(x, y) do t = max(t, abs(tx * ty));

  return t;
}

proc maxOverColumnsTailAtEnd(a : [?D] real(?w), r : [] real(w))
{
    const (R, C) = D.dims();
    const B = 0 ..< min(R.size, Y*here.maxTaskPar);

    var _c : [B, C] real(w);
    return [j in C] max reduce _c[.., j];
}

proc init1(a, r) {
  return for j in 0..<n do max reduce abs(a[.., j] * r[..]);
}

proc init2(a, r) {
  return [ j in 0..<n ] vmax(a[.., j], r[..]);
}

config param w = 64;

proc check(mask : int)
{
    const W = 0..<n;
    var a : [0..<n, 0..<n] real(w);
    var r : [0..<n] real(w);

  const alg = 1;

  var c = if alg == 0 then
    init1(a, r)
  else if alg == 1 then
    init2(a, r)
  else
    maxOverColumnsTailAtEnd(a, r);
}

config const mask = 1 | 2 | 4 | 8 | 16;

proc main
{
    check(mask);
    return(0);
}
dlongnecke-cray commented 8 months ago

Even smaller reproducer, just need an array type and iterator type on each side of the ternary:

proc test1() {
  var arr1: [0..0] int;
  var n = 0;
  var arr2 = if n == 0
    then for i in 0..0 do i
    else arr1;
}

proc main {
  test1();
}
dlongnecke-cray commented 8 months ago

Looks like there may be some unhandled cases for iterator -> array, and iterator -> iterator in insertInitConversion. My plan is to issue an _array.init= call for the first case, and promote both iterators to arrays in the second case. Both cases should probably also issue a subsequent call to insertInitConversion in order to consider array runtime types.

But first I need to determine if it's appropriate for us to be in insertInitConversion() in the first place.

dlongnecke-cray commented 8 months ago

Fixed the bug from the OP. Now I just need to get the extra case from @cassella working.

proc test1() {
  var n = 0;
  var arr = if n then for i in 0..3 do i else for i in 4..7 do i;
  writeln(arr);
}

proc main() {
  test1();
}
dlongnecke-cray commented 8 months ago

I'm going to file Paul's bug (previous post, above) as a separate issue. Initially, I was going to opt to promote the "then" branch of the if-expr to an array if both branches were iterator records, and let insertInitConversion() take care of the rest.

However, @vasslitvinov indicated to me that there might be cases where we want to preserve the "iterator'y'ness" of both sides of the if-expr. E.g., for:

foo(if n then [i in 0..3] i else [i in 4..7] i);

We could do this by creating a third iterator type which unifies both sides of the branch (forwarding to either).

Edit: Actually not sure what to do, here.

I've opened: https://github.com/chapel-lang/chapel/issues/24402