beehive-lab / TornadoVM

TornadoVM: A practical and efficient heterogeneous programming framework for managed languages
https://www.tornadovm.org
Apache License 2.0
1.18k stars 112 forks source link

[bug] Unbox nodes in GraalVM JIT IR causing compilation errors. #521

Open andrii0lomakin opened 1 month ago

andrii0lomakin commented 1 month ago

The presence of the Unbox nodes in GrallVM IR is causing compilation errors.

There are several examples of such compilation crashes from this discussion https://github.com/beehive-lab/TornadoVM/discussions/515#discussioncomment-10187348 and this one https://github.com/beehive-lab/TornadoVM/issues/516#issuecomment-2262189786 .

andrii0lomakin commented 1 month ago

Because I am working on the generation of kernels from basic functions and passed to them functional interfaces and will have to deal with this issue anyway - https://github.com/beehive-lab/TornadoVM/issues/516- I will fix this bug first if there are no objections.

andrii0lomakin commented 1 month ago

Hi @jjfumero . This is a small update from my side: I have fixed the issue that is shown in this test - https://github.com/beehive-lab/TornadoVM/issues/516#issuecomment-2262189786. I tested only on the OpenCL platform. So, I will test on all platforms before committing the changes.

I am going to add a separate test suite to test support of primitive wrappers (obviously unboxing only; no method calls are supported), and it will include the currently failing test https://github.com/beehive-lab/TornadoVM/discussions/515#discussioncomment-10187348 .

I suppose there will be other tests that need to be fixed. So, how do you prefer that I submit this fix as a separate PR or provide PR for all fixed tests?

jjfumero commented 1 month ago

Hi @andrii0lomakin , we prefer separate PRs. You can submit incremental PRs to support more and more. The important thing is that the PRs can be reproduced with tests, and those tests can run on multiple platforms (OpenCL, PTX and SPIR-V).

andrii0lomakin commented 1 month ago

@jjfumero When you mean that you prefer incremental PRs did it imply that you use some kind of PR stacking tool or resolve all upcoming changes manually?

jjfumero commented 1 month ago

I meant. every PR is one fix, in the case you have different types of fixes.