From 6e381c267cfce92b9e988c84fb3889fa6b28456c Mon Sep 17 00:00:00 2001 From: MaryXek Date: Wed, 26 Jun 2024 12:16:54 +0300 Subject: [PATCH] Apply code review comments --- .../phases/TornadoFixedArrayCopyPhase.java | 10 +++- .../drivers/ptx/graal/lir/PTXLIRStmt.java | 2 +- .../ptx/graal/nodes/FixedArrayCopyNode.java | 2 +- .../phases/TornadoFixedArrayCopyPhase.java | 18 ++++-- .../phases/TornadoFixedArrayCopyPhase.java | 57 +++++++++++++++++++ .../unittests/arrays/TestArrayCopies.java | 17 ++++++ 6 files changed, 97 insertions(+), 9 deletions(-) create mode 100644 tornado-drivers/spirv/src/main/java/uk/ac/manchester/tornado/drivers/spirv/graal/phases/TornadoFixedArrayCopyPhase.java diff --git a/tornado-drivers/opencl/src/main/java/uk/ac/manchester/tornado/drivers/opencl/graal/phases/TornadoFixedArrayCopyPhase.java b/tornado-drivers/opencl/src/main/java/uk/ac/manchester/tornado/drivers/opencl/graal/phases/TornadoFixedArrayCopyPhase.java index 753771027c..0871d0e51f 100644 --- a/tornado-drivers/opencl/src/main/java/uk/ac/manchester/tornado/drivers/opencl/graal/phases/TornadoFixedArrayCopyPhase.java +++ b/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 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 diff --git a/tornado-drivers/ptx/src/main/java/uk/ac/manchester/tornado/drivers/ptx/graal/lir/PTXLIRStmt.java b/tornado-drivers/ptx/src/main/java/uk/ac/manchester/tornado/drivers/ptx/graal/lir/PTXLIRStmt.java index cd972cc16c..4c6293b84b 100644 --- a/tornado-drivers/ptx/src/main/java/uk/ac/manchester/tornado/drivers/ptx/graal/lir/PTXLIRStmt.java +++ b/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); diff --git a/tornado-drivers/ptx/src/main/java/uk/ac/manchester/tornado/drivers/ptx/graal/nodes/FixedArrayCopyNode.java b/tornado-drivers/ptx/src/main/java/uk/ac/manchester/tornado/drivers/ptx/graal/nodes/FixedArrayCopyNode.java index aa60c25246..e0d85bee5d 100644 --- a/tornado-drivers/ptx/src/main/java/uk/ac/manchester/tornado/drivers/ptx/graal/nodes/FixedArrayCopyNode.java +++ b/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; } diff --git a/tornado-drivers/ptx/src/main/java/uk/ac/manchester/tornado/drivers/ptx/graal/phases/TornadoFixedArrayCopyPhase.java b/tornado-drivers/ptx/src/main/java/uk/ac/manchester/tornado/drivers/ptx/graal/phases/TornadoFixedArrayCopyPhase.java index ee78211a40..fa47e0e736 100644 --- a/tornado-drivers/ptx/src/main/java/uk/ac/manchester/tornado/drivers/ptx/graal/phases/TornadoFixedArrayCopyPhase.java +++ b/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 notApplicableTo(GraphState graphState) { } protected void run(StructuredGraph graph) { - ValuePhiNode phiNodeToReplace = null; + ArrayList 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,11 +73,10 @@ 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); @@ -80,6 +84,10 @@ protected void run(StructuredGraph graph) { } } + 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(); diff --git a/tornado-drivers/spirv/src/main/java/uk/ac/manchester/tornado/drivers/spirv/graal/phases/TornadoFixedArrayCopyPhase.java b/tornado-drivers/spirv/src/main/java/uk/ac/manchester/tornado/drivers/spirv/graal/phases/TornadoFixedArrayCopyPhase.java new file mode 100644 index 0000000000..240ced13c8 --- /dev/null +++ b/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 { + + @Override + public Optional 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(); + } + +} \ No newline at end of file diff --git a/tornado-unittests/src/main/java/uk/ac/manchester/tornado/unittests/arrays/TestArrayCopies.java b/tornado-unittests/src/main/java/uk/ac/manchester/tornado/unittests/arrays/TestArrayCopies.java index 2be9141b00..ea7a430353 100644 --- a/tornado-unittests/src/main/java/uk/ac/manchester/tornado/unittests/arrays/TestArrayCopies.java +++ b/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;