beehive-lab / TornadoVM

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

Support conditional copy-by-reference in local memory #462

Closed mairooni closed 1 week ago

mairooni commented 1 week ago

Description

This PR adds support for conditional copy-by-reference in local memory.

Problem description

If a plain copy-by-reference occurs in local memory, graal updates the references automatically. For example, the code below is legal and functionally correct:

public static void intPrivateCopyNoCondition(IntArray a, IntArray b) {
        for (@Parallel int i = 0; i < a.getSize(); i++) {
            int[] arrayA = new int[128];
            int[] arrayB = new int[128];
            for (int j = 0; j < 128; j++) {
                arrayA[j] = j;
                arrayB[j] = 2;
            }

            // copy arrayA to arrayB
            arrayB = arrayA;

            b.set(i, arrayB[i]);
        }
}

However, if the copy takes place depending on a condition, e.g.:

 if ((a.get(i) % 2) == 0 ) {
       // if condition is true, copy arrayA to arrayB
        arrayB = arrayA;
 }

Tornado produces a compiler error. This PR adds the necessary functionality to make this conditional copy-by-reference legal. This patch is for OpenCL and PTX, SPIR-V needs further investigation.

Backend/s tested

Mark the backends affected by this PR.

OS tested

Mark the OS where this PR is tested.

Did you check on FPGAs?

If it is applicable, check your changes on FPGAs.

How to test the new patch?

Run tornado-test -V --fast uk.ac.manchester.tornado.unittests.arrays.TestArrayCopies

jjfumero commented 1 week ago

[Review in progress] Test are passing for OpenCL and PTX.

jjfumero commented 1 week ago

Throw exception when this type of code is executed for the SPIR-V backend. This can be done within the a compilation phase.

mairooni commented 1 week ago

The comments have been addressed. @jjfumero please have another look.

jjfumero commented 1 week ago

Great. Thanks Mary!

mairooni commented 1 week ago

The refactoring has been applied, thanks @mikepapadim!

jjfumero commented 1 week ago

Great, so we merge this.