Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor common compiler phases among different backends to drivers.common.compiler.phases package and restructure compiler utilities #304

Conversation

mikepapadim
Copy link
Member

@mikepapadim mikepapadim commented Jan 8, 2024

This PR refactors common compiler phases among all backends to be moved under drivers.common package to reduce code duplication. Also, cleanups sketch tier phases under the runtime package.

Backend/s tested

Mark the backends affected by this PR.

  • OpenCL
  • PTX
  • SPIRV

OS tested

Mark the OS where this PR is tested.

  • Linux
  • OSx
  • Windows

Did you check on FPGAs?

If it is applicable, check your changes on FPGAs.

  • Yes
  • No

How to test the new patch?

## Pass Unittests using the OpenCL backend
$ make BACKEND=opencl
$ make tests

## If the changes are also applicable to the PTX backend: 
## Pass unittests using the PTX backend
$ make BACKEND=ptx
$ make tests 

## If the changes are also applicable to the SPIR-V backend: 
## Pass unittests using the SPIRV backend
$ make BACKEND=spirv
$ make tests 

Deleted duplicate BoundCheckEliminationPhase code in OpenCL, PTX, and SPIR-V modules, and created a single version in common module. This reduces redundancy and simplifies future maintenance. Updated related import statements in MidTier files accordingly.
A copyright notice has been added to the top of the BoundCheckEliminationPhase.java file. This is to establish the rights to the file, which is free software and can be modified under the terms of the GNU General Public License version 2.
Deleted TornadoPartialLoopUnroll from OpenCL and SPIRV drivers, and created a similar file under common driver. Reduced code duplication by relocating the common implementations into a shared module (drivers common). This improves maintainability and code reuse. Also updated OCLMidTier and SPIRVMidTier to include the common TornadoPartialLoopUnroll.
Simplified throwing of TornadoDeviceFP64NotSupported exception using a string literal to output device name. The refactoring also includes concise rewriting of node checking within the run function - for WriteNode, ReadNode, OCLFPUnaryIntrinsicNode, OCLFPBinaryIntrinsicNode, and SqrtNode classes.
Code inside InverseSquareRootPhase.java has been refactored to improve readability and compactness. The conditional logic checking for a ConstantNode and SqrtNode has been streamlined, resulting in cleaner, less cluttered code.
Removed unnecessary dependencies and methods in the TornadoPartialLoopUnroll class. The createLoopPolicies method and DefaultLoopPolicies import were deleted as they were surplus to requirements. Simplified the getUpperGraphLimit method by removing the unused 'graph' parameter.
The conditional structure within the put method in the PrimitiveSerialiser class was updated to a more efficient switch expression. The switch expression directly correlates buffer types with their appropriate actions, while also handling the default case of null or unsupported type.
Updated the type casting operations in the 'TornadoFieldAccessFixup.java' file to use Java's improved instance pattern matching for more straightforward and concise code. Now, each instance type check is performed simultaneously with corresponding type cast, enhancing readability and maintainability of the code.
@CLAassistant
Copy link

CLAassistant commented Jan 8, 2024

CLA assistant check
All committers have signed the CLA.

In the TornadoTaskSpecialisation class, instance type checks and casts were restructured to use Java's pattern matching for efficiency and cleaner code. This change results to inline assignments, replacing multiple lines of code. Also, switch expressions are used to simplify conditional logic, optimizing code readability and maintainability.
The TornadoNewArrayDevirtualizationReplacement.java file was repeatedly used in multiple packages. This change removes the duplicates across tornado-drivers {opencl, ptx, spirv} packages and places a single copy in the drivers-common package. The import statements referencing this file have been appropriately updated in respective packages. This eliminates redundancy and centralizes the crucial code to enhance maintainability.
All relevant compiler classes have been moved from 'graal.compiler' to 'compiler.phases'. Changes were necessary across several files and driver types, and have been adjusted accordingly. Import statements and package declarations have been updated to reflect the package location changes.
Changed module in module-info.java from module to open. This adjustment provides necessary permissions for handling reflections. Additional exports were also added in the common-exports file for the purpose of extension and API visibility.
The codebase has been refactored, particularly in 'TornadoNewArrayDevirtualizationReplacement' and 'TornadoNativeTypeElimination' class files. The refactoring involves a slight alteration in a conditional check in the type elimination process, providing improved efficiency. A missing newline character was also added at the end of the Array replacement class file.
@mikepapadim mikepapadim marked this pull request as draft January 10, 2024 11:06
@mikepapadim mikepapadim marked this pull request as draft January 10, 2024 11:06
The methods 'notApplicableTo' and 'run' in the TornadoPanamaPrivateMemory class have been reordered. This modification does not alter the program's functionality. It was done to bring the code more in line with the agreed codebase structure and improve readability.
The changes update error handling by leveraging Java's instanceof pattern matching and improve string concatenations using string templates for better readability and performance in the TornadoSketcher class. The modifications made have no functional impact on the code, but they enhance code readability and maintainability.
Removed an unnecessary variable re-initialization in the LoopCanonicalizer class. Similarly, updated the debug method in the TornadoCodeGenerator class to use TornadoLogger's static debug method, simplifying the code and potentially improving performance.
Updated TornadoTaskSpecialisation class for more readable code and potentially improved performance. The changes involve direct casting of object types in the switch-case structure instead of re-casting them again while creating constant nodes. Code readability is enhanced by clarifying variable names.
Refactored the package path for various Tornado drivers from "uk.ac.manchester.tornado.drivers.graal" to "uk.ac.manchester.tornado.drivers.providers". This includes updates to file imports across multiple classes. Moreover, syntax and spacing in some files have been altered for consistency.
The TornadoFieldAccessFixup class is moved from the 'tornado-runtime' to the shared 'drivers-common' package. Additionally, import changes were made in associated classes (SPIRVHighTier and OCLHighTier). This relocation enhances code organization and reuse. Also, minor code optimizations were implemented within the TornadoFieldAccessFixup class.
TornadoShapeAnalysis class has been moved from the `tornado.runtime.graal.phases` package to the `tornado.drivers.common.compiler.phases`. This refactoring improves the code organization by gathering all phases under the same package. Import statements and build configs were updated accordingly.
… update usages

Several files have been moved across directories and package structures have been updated in order to improve code organization. Also, multiple import statements in various files have been updated to correspond with the new location of classes and interfaces. Adjustments are made to facilitate future code maintenance and extensibility.
Removed unnecessary files, relocated some others, and adjusted the package structures to improve code organization. Updated numerous import statements to match up with the new locations of classes and interfaces. These changes will aid future maintenance and extendibility.
This commit replaces "get" methods with "getFirst" for efficiency in TornadoApiReplacement class. Strides have been replaced only in the loop condition to enhance performance. These changes potentially prevent possible runtime exceptions and ensure the smooth execution of parallelized tasks.
Removed specific exports in module info and relocated several classes in the Graal module. LoopCanonicalizer and Floatable classes along with LogicalCompareNode interface have been moved to more appropriate directories, improving code organization and hence readability. Corrections have been made to the respective import statements.
This commit refactors the way dependencies are set in IntermediateTornadoGraph. It utilizes the newly introduced pattern matching for instanceof in Java to simplify and clarify the code, particularly in handling DependentReadNode.
@mikepapadim mikepapadim marked this pull request as ready for review January 11, 2024 09:32
@mikepapadim
Copy link
Member Author

This is ready for review

@jjfumero jjfumero changed the base branch from master to develop January 11, 2024 09:34
@mikepapadim mikepapadim changed the title Refactor duplicate compiler phases from opencl,ptx,spirv-drivers to drivers.common Refactor common compiler phases to drivers.common.compiler and move non-sketcher phases from runtime Jan 11, 2024
The 'sketch.tier' package name has been renamed to 'sketcher'.
@mikepapadim mikepapadim changed the title Refactor common compiler phases to drivers.common.compiler and move non-sketcher phases from runtime Refactor common compiler phases among different backends to drivers.common.compiler.phases package and restructure compiler utilities Jan 11, 2024
Copy link
Collaborator

@mairooni mairooni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, APT Group, Department of Computer Science,
* Copyright (c) 2023, APT Group, Department of Computer Science,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Copyright (c) 2023, APT Group, Department of Computer Science,
* Copyright (c) 2020, 2024 APT Group, Department of Computer Science,

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, APT Group, Department of Computer Science,
* Copyright (c) 2023, APT Group, Department of Computer Science,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Copyright (c) 2023, APT Group, Department of Computer Science,
* Copyright (c) 2022, 2024 APT Group, Department of Computer Science,

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, APT Group, Department of Computer Science,
* Copyright (c) 2023, APT Group, Department of Computer Science,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Copyright (c) 2023, APT Group, Department of Computer Science,
* Copyright (c) 2020, 2024 APT Group, Department of Computer Science,

Please, review all header files with the license to fix with the correct year.

* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*/
* This file is part of Tornado: A heterogeneous programming framework:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Livence is not the same. Check this file

* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
*/
* This file is part of Tornado: A heterogeneous programming framework:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the license.

@jjfumero
Copy link
Member

All unitests passing + KFusion + RayTracer

…tching

The code is refactored to replace traditional switch-case statements with the enhanced pattern matching provided in later versions of Java. This change extends across multiple files and functions, providing more concise and readable code. Additionally, the GRAPH_NODES_UPPER_LIMIT value is now a constant, improving the code maintainability.
Switch-case statements in OCLWriteAtomicNode have been refactored to use pattern matching. This change provides the dual benefits of making the code more readable and concise. Additionally, it addresses PR comments and increases maintainability by defining the GRAPH_NODES_UPPER_LIMIT as a constant value.
This commit simplifies the switch statements in the SPIRVFPBinaryIntrinsicNode class.
The code has been refactored to use the new, more concise style of Java switch expressions, improving readability. This refactoring affects the 'doCompute' and 'generate' methods in the SPIRVIntrinsicNode classes. The use of explicit break statements has been phased out, making the code cleaner and easier to understand.
The copyright years in various files in drivers-common were updated to include 2024. This includes files in the compiler, memalloc, analysis, loops, and guards subdirectories. Always remember to update copyright notices at the start of each new year for accurate documentation.
The copyright dates in Tornado runtime have been updated across various files. This covers
The copyright year has been updated from 2020 and 2021 to 2024 in numerous files within the Tornado runtime. This change ensures that the copyright information accurately reflects the current year.
@jjfumero
Copy link
Member

LGTM. thanks @mikepapadim

@jjfumero jjfumero merged commit 559fa45 into beehive-lab:develop Jan 12, 2024
2 checks passed
@mikepapadim mikepapadim deleted the mikepapadim/refactor_common_compiler_infra branch January 12, 2024 11:24
jjfumero added a commit that referenced this pull request Jan 30, 2024
TornadoVM 1.0.1
----------------
30/01/2024

Improvements
~~~~~~~~~~~~~~~~~~

- `#305 <https://github.com/beehive-lab/TornadoVM/pull/305>`_: Under-demand data transfer for custom data ranges.
- `#305 <https://github.com/beehive-lab/TornadoVM/pull/305>`_: Copy out subregions using the execution plan:
- `#313 <https://github.com/beehive-lab/TornadoVM/pull/313>`_: Initial support for Half-Precision (FP16) data types.
- `#311 <https://github.com/beehive-lab/TornadoVM/pull/311>`_: Enable Multi-Task Multiple Device (MTMD) model from the ``TornadoExecutionPlan`` API:
- `#315 <https://github.com/beehive-lab/TornadoVM/pull/315>`_: Math ``Ceil`` function added

Compatibility/Integration
~~~~~~~~~~~~~~~~~~~~~~~~~~~

- `#294 <https://github.com/beehive-lab/TornadoVM/pull/294>`_: Separation of the OpenCL Headers from the code base.
- `#297 <https://github.com/beehive-lab/TornadoVM/pull/297>`_: Separation of the LevelZero JNI API in a separate repository.
- `#301 <https://github.com/beehive-lab/TornadoVM/pull/301>`_: Temurin configuration supported.
- `#304 <https://github.com/beehive-lab/TornadoVM/pull/304>`_: Refactor of the common phases for the JIT compiler.
- `#316 <https://github.com/beehive-lab/TornadoVM/pull/316>`_: Beehive SPIR-V Toolkit version updated.

Bug Fixes
~~~~~~~~~~~~~~~~~~

- `#298 <https://github.com/beehive-lab/TornadoVM/pull/298>`_: OpenCL Codegen fixed open-close brackets.
- `#300 <https://github.com/beehive-lab/TornadoVM/pull/300>`_: Python Dependencies fixed for AWS
- `#308 <https://github.com/beehive-lab/TornadoVM/pull/308>`_: Runtime check for Grid-Scheduler names
- `#309 <https://github.com/beehive-lab/TornadoVM/pull/309>`_: Fix check-style to support STR templates
- `#314 <https://github.com/beehive-lab/TornadoVM/pull/314>`_: emit Vector16 Capability for 16-width vectors
jjfumero added a commit that referenced this pull request Jan 30, 2024
Improvements
~~~~~~~~~~~~~~~~~~

- `#305 <https://github.com/beehive-lab/TornadoVM/pull/305>`_: Under-demand data transfer for custom data ranges.
- `#313 <https://github.com/beehive-lab/TornadoVM/pull/313>`_: Initial support for Half-Precision (FP16) data types.
- `#311 <https://github.com/beehive-lab/TornadoVM/pull/311>`_: Enable Multi-Task Multiple Device (MTMD) model from the ``TornadoExecutionPlan`` API.
- `#315 <https://github.com/beehive-lab/TornadoVM/pull/315>`_: Math ``Ceil`` function added.

Compatibility/Integration
~~~~~~~~~~~~~~~~~~~~~~~~~~~

- `#294 <https://github.com/beehive-lab/TornadoVM/pull/294>`_: Separation of the OpenCL Headers from the code base.
- `#297 <https://github.com/beehive-lab/TornadoVM/pull/297>`_: Separation of the LevelZero JNI API in a separate repository.
- `#301 <https://github.com/beehive-lab/TornadoVM/pull/301>`_: Temurin configuration supported.
- `#304 <https://github.com/beehive-lab/TornadoVM/pull/304>`_: Refactor of the common phases for the JIT compiler.
- `#316 <https://github.com/beehive-lab/TornadoVM/pull/316>`_: Beehive SPIR-V Toolkit version updated.

Bug Fixes
~~~~~~~~~~~~~~~~~~

- `#298 <https://github.com/beehive-lab/TornadoVM/pull/298>`_: OpenCL Codegen fixed open-close brackets.
- `#300 <https://github.com/beehive-lab/TornadoVM/pull/300>`_: Python Dependencies fixed for AWS.
- `#308 <https://github.com/beehive-lab/TornadoVM/pull/308>`_: Runtime check for Grid-Scheduler names.
- `#309 <https://github.com/beehive-lab/TornadoVM/pull/309>`_: Fix check-style to support STR templates.
- `#314 <https://github.com/beehive-lab/TornadoVM/pull/314>`_: emit Vector16 Capability for 16-width vectors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

5 participants