Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vm] OSR missing phis #45260

Closed
dcharkes opened this issue Mar 9, 2021 · 1 comment
Closed

[vm] OSR missing phis #45260

dcharkes opened this issue Mar 9, 2021 · 1 comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi

Comments

@dcharkes
Copy link
Contributor

dcharkes commented Mar 9, 2021

/===========================================================================================\
| ffi_2/inline_array_multi_dimensional_test is new and failed (RuntimeError, expected Pass) |
\===========================================================================================/

--- Command "vm" (took 01.000672s):
DART_CONFIGURATION=ReleaseX64 out/ReleaseX64/dart --optimization-counter-threshold=5 --random-seed=1974046516 --ignore-unrecognized-flags --packages=/b/s/w/ir/.packages /b/s/w/ir/tests/ffi_2/inline_array_multi_dimensional_test.dart

exit code:
255

stderr:
Unhandled exception:
Expect.equals(at index 38: Expected <...([[[[[0, 1], [2, 3]], [[4, 5], [6, 7]]], [[[8, 9], [10, 11]], [[12, 13], [14, 15]]]], [[[[16, 17], [18, 19]], [[20, 21], [22, 23]]], [[[24, 25], [26, 27]], [[28, 29], [30, 31]]]]])...>, Found: <...([[[[[0, 1], [2, 3]], [[4, 5], [6, 7]], [[8, 9], [10, 11]], [[12, 13], [14, 15]], [[16, 17], [18, 19]], [[20, 21], [22, 23]], [[24, 25], [26, 27]], [[28, 29], [30, 31]]], [], [], []], []])...>) fails.
#0      Expect._fail (package:expect/expect.dart:702:5)
#1      Expect.equals (package:expect/expect.dart:127:9)
#2      testToString (file:///b/s/w/ir/tests/ffi_2/inline_array_multi_dimensional_test.dart:141:10)
#3      main (file:///b/s/w/ir/tests/ffi_2/inline_array_multi_dimensional_test.dart:20:3)
#4      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:283:19)
#5      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:184:12)

--- Re-run this test:
python tools/test.py -n dartk-optcounter-linux-release-x64 ffi_2/inline_array_multi_dimensional_test

looks like some index calls are not happening: , [], [], []], []]).

https://dart-ci.appspot.com/log/any/dartk-optcounter-linux-release-x64/12076/ffi_2/inline_array_multi_dimensional_test

@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi labels Mar 9, 2021
@dcharkes
Copy link
Contributor Author

dcharkes commented Apr 21, 2021

Non-ffi regress test: https://dart-review.googlesource.com/c/sdk/+/196320/2/tests/language/vm/regress_45260_test.dart

Expected result:

const expectedList = [
  [
    [
      [0, 1],
      [2, 3]
    ],
    [
      [4, 5],
      [6, 7]
    ]
  ],
  [
    [
      [8, 9],
      [10, 11]
    ],
    [
      [12, 13],
      [14, 15]
    ]
  ]
];

Actual result:

const expectedList = [
  [
    [
      [0, 1],
      [2, 3]
    ],
    [
      [4, 5],
      [6, 7]
    ],
    [ // The add of this sublist is to the wrong surrounding list.
      [8, 9],
      [10, 11]
    ],
    [ // The add of this sublist is to the wrong surrounding list.
      [12, 13],
      [14, 15]
    ]
  ],
  [] // They should have been added to this list.
];

The StoreLocal to :var1 in B3 is not picked up by the LoadLocal in B5 after OSR.

Unoptimized IL gist.

Optimized OSR IL gist.

Optimized non-OSR IL gist, renamed variables to match OSR IL.

The add methods are invoked on the wrong list because only the OSR param holding the at-the-OSR-time t1 stack variable flows into the add. The definition from the new list for :var1 from B3 and loaded in B5 is never seen in B12 because B12 does not have a phi for t1 and consequently always reads the original t1 from the OSR-entry.

The synthetic phis inserted for the OSR entry are only inserted for the first join after the OSR entry. This is wrong, we should have had a phi in B12 for the t1 stack variable.

Image of the various IL graphs (blue is unoptimized, and optimized non-OSR, gray is optimized OSR):
image

@dcharkes dcharkes changed the title [vm/ffi] optcounter failure on inline_array_multi_dimensional_test [vm] OSR missing phis Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi
Projects
None yet
Development

No branches or pull requests

1 participant