Move Interpreter to test area

[mabbay]

We move Interpreter to the location of runtime tests alongside a copy of ArithmeticAndConvOpImpls. Runtime tests that depends on Interpreter refer to it via @library tag.
The original copy of ArithmeticAndConvOpImpls is kept in the internal package as it's used by op evaluation API. We remove methods from internal.ArithmeticAndConvOpImpls that are not used by the op evaluation API.

---

Progress

• Change must not contain extraneous whitespace

Reviewers

• Maurizio Cimadamore (@mcimadamore - Reviewer) ⚠️ Review applies to c71e1103

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/babylon.git pull/969/head:pull/969
$ git checkout pull/969

Update a local copy of the PR:
$ git checkout pull/969
$ git pull https://git.openjdk.org/babylon.git pull/969/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 969

View PR using the GUI difftool:
$ git pr show -t 969

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/babylon/pull/969.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back mabbay! A progress list of the required criteria for merging this PR into code-reflection will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@mabbay This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

Move Interpreter to test area

Reviewed-by: mcimadamore

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 6 new commits pushed to the code-reflection branch:

• 2916f59: fix step and allow width/height to differ - galaxy shader
• 919206c: [hat] fix flag to show kernel model
• 1dc818e: Various fixes to java side and opencl mapped vecs and mats
• ... and 3 more: https://git.openjdk.org/babylon/compare/dce0e05f83d120ede89d27aef8279e7dfa2c95bc...code-reflection

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the code-reflection branch, type /integrate in a new comment.

[openjdk]

@mabbay this pull request can not be integrated into code-reflection due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout mv-interpreter
git fetch https://git.openjdk.org/babylon.git code-reflection
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge code-reflection"
git push

[mlbridge]

Webrevs

• 02: Full - Incremental (b0ea1021)
• 01: Full - Incremental (7522492d)
• 00: Full (c71e1103)

[mcimadamore]

I tried locally -- and effectively these seem to work fine -- I remember I excluded them because otherwise they would fail because of the testng imports -- but they look fine now. Odd.

[mabbay]

Maybe because we migrated from TestNG to JUnit

[mcimadamore]

Likely

[mcimadamore]

Overall looks good. The only doubt I have is whether we should really duplicate the ArithmeticAndConvOp class. Note that we can use the @modules tag to give test access to non-exported package:

@modules jdk.incubator.code/<package name>

This would allow to keep only one copy (and keep it in a non-exported package)

[mabbay]

Overall looks good. The only doubt I have is whether we should really duplicate the ArithmeticAndConvOp class. Note that we can use the @modules tag to give test access to non-exported package:

@modules jdk.incubator.code/<package name>

This would allow to keep only one copy (and keep it in a non-exported package)

ArithmeticAndConvOp used by Interpreter has few methods more then the class used by operation evaluation API.

[mcimadamore]

For now looks ok -- if we notice that code starts diverging too much, or that we need to keep the internal version of ArithmenticAndConvOp in sync with the test/interpreter one, we might revisit.

[PaulSandoz]

For now looks ok -- if we notice that code starts diverging too much, or that we need to keep the internal version of ArithmenticAndConvOp in sync with the test/interpreter one, we might revisit.

The divergence is due to the lifter of bytecode to code models, where the use of primitive conversion operation is extended to support conversion between boolean values, and then the Interpreter is used to test such lifted models. We need to revisit the lifter and also leave appropriate comments to remind us.

[PaulSandoz]

Can you add a comment as to why we need to duplicate and extend the non-test version?

[mabbay]

/integrate

[openjdk]

Going to push as commit 2075f10.
Since your change was applied there have been 6 commits pushed to the code-reflection branch:

• 2916f59: fix step and allow width/height to differ - galaxy shader
• 919206c: [hat] fix flag to show kernel model
• 1dc818e: Various fixes to java side and opencl mapped vecs and mats
• ... and 3 more: https://git.openjdk.org/babylon/compare/dce0e05f83d120ede89d27aef8279e7dfa2c95bc...code-reflection

Your commit was automatically rebased without conflicts.

[openjdk]

@mabbay Pushed as commit 2075f10.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.