Apply code review comments

tornado-drivers/opencl/src/main/java/uk/ac/manchester/tornado/drivers/opencl/graal/phases/TornadoFixedArrayCopyPhase.java

@@ -29,6 +29,7 @@
 import org.graalvm.compiler.nodes.memory.address.OffsetAddressNode;
 import org.graalvm.compiler.phases.Phase;
 
+import uk.ac.manchester.tornado.api.exceptions.TornadoCompilationException;
 import uk.ac.manchester.tornado.drivers.opencl.graal.OCLArchitecture;
 import uk.ac.manchester.tornado.drivers.opencl.graal.nodes.FixedArrayCopyNode;
 import uk.ac.manchester.tornado.drivers.opencl.graal.nodes.FixedArrayNode;
@@ -48,7 +49,7 @@ public Optional<NotApplicable> notApplicableTo(GraphState graphState) {
 
     protected void run(StructuredGraph graph) {
         for (ValuePhiNode phiNode : graph.getNodes().filter(ValuePhiNode.class)) {
-            if (phiNode.usages().filter(OffsetAddressNode.class).isNotEmpty() && phiNode.values().filter(FixedArrayNode.class).isNotEmpty()) {
+            if (isFixedArrayCopied(phiNode)) {
                 FixedArrayNode fixedArrayNode = phiNode.values().filter(FixedArrayNode.class).first();
                 ResolvedJavaType resolvedJavaType = fixedArrayNode.getElementType();
                 OCLArchitecture.OCLMemoryBase oclMemoryBase = fixedArrayNode.getMemoryRegister();
@@ -58,12 +59,17 @@ protected void run(StructuredGraph graph) {
                 offsetAddressNode.replaceFirstInput(phiNode, fixedArrayCopyNode);
                 // finally, since we know that the data accessed is a fixed array, fix the offset
                 ValuePhiNode privateIndex = getPrivateArrayIndex(offsetAddressNode.getOffset());
+                if (privateIndex == null) {
+                    throw new TornadoCompilationException("Index of FixedArrayNode is null.");
+                }
                 offsetAddressNode.setOffset(privateIndex);
-                break;
             }
         }
     }
 
+    private static boolean isFixedArrayCopied(ValuePhiNode phiNode) {
+        return phiNode.usages().filter(OffsetAddressNode.class).isNotEmpty() && phiNode.values().filter(FixedArrayNode.class).isNotEmpty();
+    }
 
     private static ValuePhiNode getPrivateArrayIndex(Node node) {
         // identify the index

tornado-drivers/ptx/src/main/java/uk/ac/manchester/tornado/drivers/ptx/graal/lir/PTXLIRStmt.java

@@ -886,7 +886,7 @@ public void emitCode(PTXCompilationResultBuilder crb, PTXAssembler asm) {
             asm.delimiter();
             asm.eol();
             // add base with offset
-            // since local memory base is always, u32 is hardcoded
+            // since local memory base is always an unsigned int, u32 is hardcoded
             asm.emitSymbol(TAB);
             asm.emit(ADD + DOT + "u32");
             asm.emitSymbol(SPACE);

tornado-drivers/ptx/src/main/java/uk/ac/manchester/tornado/drivers/ptx/graal/nodes/FixedArrayCopyNode.java

@@ -120,7 +120,7 @@ public void generate(NodeLIRBuilderTool gen) {
             result = tool.newVariable(lirKind);
             tool.append(new PTXLIRStmt.AssignStmt(result, new PTXUnary.Expr(PTXAssembler.PTXUnaryOp.CVT_FLOAT, lirKind, baseAddress)));
             declarationPtr = new PTXBinary.Expr(pointerCopyTemplate, lirKind, result, baseAddress);
-    } else {
+        } else {
             declarationPtr = new PTXBinary.Expr(pointerCopyTemplate, lirKind, offsetedIndex, baseAddress);
             result = offsetedIndex;
         }

tornado-drivers/ptx/src/main/java/uk/ac/manchester/tornado/drivers/ptx/graal/phases/TornadoFixedArrayCopyPhase.java

@@ -30,12 +30,14 @@
 import org.graalvm.compiler.nodes.memory.address.OffsetAddressNode;
 import org.graalvm.compiler.phases.Phase;
 
+import uk.ac.manchester.tornado.api.exceptions.TornadoCompilationException;
 import uk.ac.manchester.tornado.drivers.ptx.graal.PTXArchitecture;
 import uk.ac.manchester.tornado.drivers.ptx.graal.lir.PTXKind;
 import uk.ac.manchester.tornado.drivers.ptx.graal.nodes.FixedArrayCopyNode;
 import uk.ac.manchester.tornado.drivers.ptx.graal.nodes.FixedArrayNode;
 import uk.ac.manchester.tornado.drivers.ptx.graal.PTXStampFactory;
 
+import java.util.ArrayList;
 import java.util.Optional;
 
 /**
@@ -50,15 +52,18 @@ public Optional<NotApplicable> notApplicableTo(GraphState graphState) {
     }
 
     protected void run(StructuredGraph graph) {
-        ValuePhiNode phiNodeToReplace = null;
+        ArrayList<ValuePhiNode> phiNodesToReplace = new ArrayList<>();
         ResolvedJavaType resolvedJavaType = null;
         for (ValuePhiNode phiNode : graph.getNodes().filter(ValuePhiNode.class)) {
-            if (phiNode.usages().filter(OffsetAddressNode.class).isNotEmpty() && phiNode.values().filter(FixedArrayNode.class).isNotEmpty()) {
+            if (isFixedArrayCopied(phiNode)) {
                 FixedArrayNode fixedArrayNode = phiNode.values().filter(FixedArrayNode.class).first();
                 resolvedJavaType = fixedArrayNode.getElementType();
                 PTXArchitecture.PTXMemoryBase oclMemoryBase = fixedArrayNode.getMemoryRegister();
                 OffsetAddressNode offsetAddressNode = phiNode.usages().filter(OffsetAddressNode.class).first();
                 ValuePhiNode privateIndex = getPrivateArrayIndex(offsetAddressNode.getOffset());
+                if (privateIndex == null) {
+                    throw new TornadoCompilationException("Index of FixedArrayNode is null.");
+                }
                 FixedArrayCopyNode fixedArrayCopyNode = new FixedArrayCopyNode(phiNode, resolvedJavaType, oclMemoryBase, privateIndex);
                 graph.addWithoutUnique(fixedArrayCopyNode);
                 offsetAddressNode.replaceFirstInput(phiNode, fixedArrayCopyNode);
@@ -68,18 +73,21 @@ protected void run(StructuredGraph graph) {
                     deleteFixed(readNode);
                     offsetAddressNode.safeDelete();
                 }
-                phiNodeToReplace = phiNode;
-                break;
+                phiNodesToReplace.add(phiNode);
             }
         }
-        if (phiNodeToReplace != null) {
+        for (ValuePhiNode phiNodeToReplace : phiNodesToReplace) {
             ValuePhiNode newPhiNode = new ValuePhiNode(PTXStampFactory.getStampFor(PTXKind.U32), phiNodeToReplace.merge(), phiNodeToReplace.valueAt(0), phiNodeToReplace.valueAt(1));
             graph.addWithoutUnique(newPhiNode);
             phiNodeToReplace.replaceAtUsages(newPhiNode);
             phiNodeToReplace.safeDelete();
         }
     }
 
+    private static boolean isFixedArrayCopied(ValuePhiNode phiNode) {
+        return phiNode.usages().filter(OffsetAddressNode.class).isNotEmpty() && phiNode.values().filter(FixedArrayNode.class).isNotEmpty();
+    }
+
     private static void deleteFixed(Node n) {
         Node pred = n.predecessor();
         Node suc = n.successors().first();

tornado-drivers/spirv/src/main/java/uk/ac/manchester/tornado/drivers/spirv/graal/phases/TornadoFixedArrayCopyPhase.java

@@ -0,0 +1,57 @@
+/*
+ * This file is part of Tornado: A heterogeneous programming framework:
+ * https://github.com/beehive-lab/tornadovm
+ *
+ * Copyright (c) 2024, APT Group, Department of Computer Science,
+ * School of Engineering, The University of Manchester. All rights reserved.
+ * Copyright (c) 2009-2021, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+package uk.ac.manchester.tornado.drivers.spirv.graal.phases;
+
+import org.graalvm.compiler.nodes.GraphState;
+import org.graalvm.compiler.nodes.StructuredGraph;
+import org.graalvm.compiler.nodes.ValuePhiNode;
+import org.graalvm.compiler.nodes.memory.address.OffsetAddressNode;
+import org.graalvm.compiler.phases.BasePhase;
+
+import uk.ac.manchester.tornado.runtime.graal.phases.TornadoLowTierContext;
+import uk.ac.manchester.tornado.drivers.spirv.graal.nodes.FixedArrayNode;
+
+import java.util.Optional;
+
+public class TornadoFixedArrayCopyPhase extends BasePhase<TornadoLowTierContext> {
+
+    @Override
+    public Optional<NotApplicable> notApplicableTo(GraphState graphState) {
+        return ALWAYS_APPLICABLE;
+    }
+
+    protected void run(StructuredGraph graph, TornadoLowTierContext context) {
+        for (ValuePhiNode phiNode : graph.getNodes().filter(ValuePhiNode.class)) {
+            if (isFixedArrayCopied(phiNode)) {
+                throw new TornadoCompilationException("Copying a local array by reference is not currently supported for SPIR-V.");
+            }
+        }
+    }
+
+    private static boolean isFixedArrayCopied(ValuePhiNode phiNode) {
+        return phiNode.usages().filter(OffsetAddressNode.class).isNotEmpty() && phiNode.values().filter(FixedArrayNode.class).isNotEmpty();
+    }
+
+}

tornado-unittests/src/main/java/uk/ac/manchester/tornado/unittests/arrays/TestArrayCopies.java

@@ -1,3 +1,20 @@
+/*
+ * Copyright (c) 2024, APT Group, Department of Computer Science,
+ * The University of Manchester.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
 package uk.ac.manchester.tornado.unittests.arrays;
 
 import org.junit.Test;