Sitelet https://web.archive.org/web/20240911083522/https://github.com/ponylang/ponyc/issues/3857
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.

Sign up for GitHub

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

Jump to bottom

Remove usage of JIT for tests #3857

Closed
SeanTAllen opened this issue Sep 16, 2021 · 12 comments · Fixed by #3907
Closed

Remove usage of JIT for tests #3857

SeanTAllen opened this issue Sep 16, 2021 · 12 comments · Fixed by #3907
Assignees
@chalcolith @SeanTAllen
Labels
good first issue Good for newcomers
Milestone
Arm support

Comments

@SeanTAllen
Copy link
Member

SeanTAllen commented Sep 16, 2021 •
edited
Loading

Currently we have a number of tests for functionality that use the JIT to run compiled code. This is handy for tests, however, the ORC JIT is still a "very moving" target. Additionally, it appears that it is buggy (or our usage of it is buggy) on Raspberry PI 4 32-bit which is one of our current Arm development platforms.

We've tentatively decided we will remove JIT usage from the codebase and switch to using *.pony source file programs that a freshly built ponyc can compile and then a test harness can run and compare the expected exit code to the actual exit code. That would cover what all the JIT based tests do.

Here's an example test from codegen.cc:

TEST_F(CodegenTest, JitRun)
{
  const char* src =
    "use @pony_exitcode[None](code: I32)\n"

    "actor Main\n"
    "  new create(env: Env) =>\n"
    "    @pony_exitcode(1)";

  TEST_COMPILE(src);

  int exit_code = 0;
  ASSERT_TRUE(run_program(&exit_code));
  ASSERT_EQ(exit_code, 1);
}

All the JIT tests can be identify by the call to run_program.

I would suggest starting with two tests to get working and see how difficult this will be.

  • JitRun which should be the easiest of any to move over
  • CCallback which calls into custom C code via FFI and will involve far more moving parts.
@SeanTAllen SeanTAllen added help wanted Extra attention is needed good first issue Good for newcomers labels Sep 16, 2021
@SeanTAllen SeanTAllen mentioned this issue Sep 17, 2021
Closed
@SeanTAllen
Copy link
Member Author

As part of this, add a regression test for #2976:

actor Main
  new create(env: Env) =>
    apply(env)
  
  fun apply(env: Env) =>
    test[I64](env, (0, true))
    test[U64](env, (0, true))
  
  fun test[A: Stringable #read](env: Env, expected: (A, Bool)) =>
    busywork[A](env, expected._1)
  
  fun busywork[A: Stringable #read](env: Env, expected: A) =>
    "." + "." + "." + "." + "." + "." + "." + "." + "." + "." + "." + "." + "."

@SeanTAllen
Copy link
Member Author

There's a "it could look like this" draft PR open:

#3865

@SeanTAllen SeanTAllen added this to the Arm support milestone Sep 24, 2021
@SeanTAllen SeanTAllen mentioned this issue Sep 24, 2021
Closed
@SeanTAllen SeanTAllen removed the help wanted Extra attention is needed label Oct 3, 2021
@SeanTAllen
Copy link
Member Author

At the moment, this is waiting for @kulibali's word on if it makes sense to integrate these into cmake build system (which we hope it does).

@chalcolith
Copy link
Member

@SeanTAllen I've committed some preliminary work in no-jit-tests-kulibali which is off of your no-jit-tests branch. In it I build the "associated" libraries for the jit tests during the cmake build (they end up in build/{release,debug}/test_lib). Then the Makefile (and make.ps1 could call a runner to compile and run the tests). I've just started such a runner (in Pony, because why not).

Note that the tests will need to be modified to link to libraries that are named after them (e.g. c-callback.lib).

@SeanTAllen
Copy link
Member Author

@kulibali is the plan for you to get everything on that branch to working and then i can take over again on trying to fit the rest of the more complicated tests in?

@chalcolith
Copy link
Member

I was thinking you could see if it's an approach you like and then cherry-pick from my branch what you want to include.

@SeanTAllen
Copy link
Member Author

@kulibali I'd rather work together on your branch together. Whatever either of us doesn't like, we can talk and adjust it.

Does that work for you? If yes, I'd say, let me know when you are ready for me to do whatever my next thing is and let me know when its time and I'll start figuring out the state of things.

@SeanTAllen
Copy link
Member Author

@kulibali excellent idea to write the runner in Pony. very excellent idea.

@chalcolith
Copy link
Member

There's some work in my branch no-jit-tests-kulibali now. The runner doesn't work right yet, but its fully integrated into the build system. make build will build the runner and the "additional" libraries that the tests need, then the runner tries to build and run the pony program in each test directory. Do runner --help to see lots of options.

I'll keep working on the runner.

@SeanTAllen
Copy link
Member Author

Let me know when you want me to jump in @kulibali.

@jemc
Copy link
Member

jemc commented Oct 19, 2021

@kulibali says the new runner is working and the next step is to use the new runner to convert all the tests.

@kulibali and @SeanTAllen will tag team on converting the tests.

@SeanTAllen
Copy link
Member Author

We have 3 tests left to convert and probably some housekeeping around output format and what not to deal with.

Those 3 tests that are left are rather tricky compared to all the others that were straightforward to do.

@chalcolith chalcolith mentioned this issue Oct 27, 2021
Merged
SeanTAllen pushed a commit that referenced this issue Dec 10, 2021
@chalcolith
Remove usage of JIT tests (#3907)
6a1a93e 
Previously there were a number of tests in `tests/libponyc` that relied on the LLVM ORC JIT to test Pony compilation. Several of these never worked right on Windows or ARM.

This PR (a collaboration between @SeanTAllen and @kulibali) removes all tests that depend on the JIT from `tests/libponyc` and moves them to individual directories in `tests/libponyc-run`.

Updates the build system to build individual libraries from any C or C++ code found in subdirectories of `tests/libponyc-run` when running `make build` (`make.ps1 build` on Windows). These libraries end up in `build/{release,debug}`.

Adds a program in `tests/libponyc-run/runner` that, during `make test`, does the following for each subdirectory in `tests/libponyc-run` that contains Pony code:

- Runs `ponyc` on that directory, linking with the libraries built by `make build` as necessary.
- Runs the resulting program, comparing the exit code with the value in `expected-exit-code.txt` (or `0` if that file does not exist).

Fixes #3857
Fixes #2455 (the exception-catching test now works)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chalcolith chalcolith

@SeanTAllen SeanTAllen

Labels
good first issue Good for newcomers
Projects
None yet
Milestone
Arm support
Development

Successfully merging a pull request may close this issue.

Remove usage of JIT tests ponylang/ponyc
3 participants
@chalcolith @SeanTAllen @jemc