search

chapel-lang / chapel

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

RV failure for test_zipper_range #11833

Closed mppf closed 5 years ago

mppf commented 5 years ago

RV integration causes correctness failures for test/distributions/robust/arithmetic/basics/test_zipper_range.chpl

Here is a small reproducer:

var A1D:[1..100] real;

proc kernel(A) {
  forall (a,i) in zip(A1D, 1.. by 2) do
    a += 2.0*i;
}

kernel(A1D);
writeln(A1D);

Compile it (after PR #11779 and after cd third-party/llvm && ./update-llvm.sh master && cd ../.. && make) with

chpl --fast --llvm program.chpl

The expected output for this program is

2.0 6.0 10.0 14.0 18.0 22.0 26.0 30.0 34.0 38.0 42.0 46.0 50.0 54.0 58.0 62.0 66.0 70.0 74.0 78.0 82.0 86.0 90.0 94.0 98.0 102.0 106.0 110.0 114.0 118.0 122.0 126.0 130.0 134.0 138.0 142.0 146.0 150.0 154.0 158.0 162.0 166.0 170.0 174.0 178.0 182.0 186.0 190.0 194.0 198.0 202.0 206.0 210.0 214.0 218.0 222.0 226.0 230.0 234.0 238.0 242.0 246.0 250.0 254.0 258.0 262.0 266.0 270.0 274.0 278.0 282.0 286.0 290.0 294.0 298.0 302.0 306.0 310.0 314.0 318.0 322.0 326.0 330.0 334.0 338.0 342.0 346.0 350.0 354.0 358.0 362.0 366.0 370.0 374.0 378.0 382.0 386.0 390.0 394.0 398.0

but on my system, the RV-optimized version emits

0.0 0.0 0.0 2.0 4.0 4.0 4.0 6.0 8.0 8.0 8.0 10.0 50.0 0.0 0.0 0.0 54.0 4.0 4.0 4.0 58.0 86.0 90.0 94.0 98.0 0.0 0.0 0.0 102.0 4.0 4.0 4.0 106.0 8.0 8.0 8.0 110.0 150.0 0.0 0.0 0.0 154.0 4.0 4.0 4.0 158.0 186.0 190.0 194.0 198.0 0.0 0.0 0.0 202.0 4.0 4.0 4.0 206.0 8.0 8.0 8.0 210.0 250.0 0.0 0.0 0.0 254.0 4.0 4.0 4.0 258.0 286.0 290.0 294.0 298.0 0.0 0.0 0.0 302.0 4.0 4.0 4.0 306.0 8.0 8.0 8.0 310.0 350.0 0.0 0.0 0.0 354.0 4.0 4.0 4.0 358.0 386.0 390.0 394.0 398.0

I think there is a problem with the initial value for the vectorized version of the loop variable representing the index in 1.. by 2.

simoll commented 5 years ago

The zipper test works now https://github.com/cdl-saarland/rv/commit/0725e5e22d8129e3fa984147cae80651e34ff52c .. by not vectorizing. As of this writing, RV can not handle reduction SCCs (value reductions) that have users inside the loop.

mppf commented 5 years ago

@simoll - is this pattern hard enough for RV to support that the Chapel front end should avoid hinting the loop as vectorizeable with rv.vectorize.enable?

simoll commented 5 years ago

@mppf - it is possible to vectorize this (leave the phi scalar, use prefix sum to build the vector for in-loop users, reduce to scalar in the loop latch). This is just not implemented yet. Given that the hint doesn't do any harm, i see no reason to remove it. RV will vectorize the loop once this pattern is supported.

mppf commented 5 years ago

@simoll - OK, thanks. I'll close this issue since your fix is working for me & there's no further action required on the Chapel front-end side.