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

[API] Extension of the prebuilt-API to load kernels from JAR files #505

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

jjfumero
Copy link
Member

@jjfumero jjfumero commented Jul 25, 2024

Description

Extension of the prebuilt API to be able to load prebuilt kernels stored in JAR files.

Example:

TaskGraph taskGraph = new TaskGraph("sample")
   .transferToDevice(DataTransferMode.EVERY_EXECUTION, param1, param2, ... )
         .prebuiltTask("t1", // task name
                        "myKernel", // kernel name
                        MyKlass.class,    // Accessible klass in the CLASSAPATH that can access the ptx file.
			"kernelFile.ptx",  // File that contains the kernel to run
			accessorParameters)  
                .transferToHost(DataTransferMode.EVERY_EXECUTION, output);

This API access is required by GAIA for the project AERO.

Problem description

n/ a.

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?

tornado-test -V uk.ac.manchester.tornado.unittests.prebuilt.PrebuiltTest#testPrebuiltWithJar

This has been tested in the GAIA repo with the following commnad:

tornado --threadInfo -cp target/gaia-1.0-SNAPSHOT.jar gaia.MethodLeastSquareTornado 13

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.

LGTM

@stratika
Copy link
Collaborator

@jjfumero is it possible to add a small test case that uses this API extension?

@jjfumero
Copy link
Member Author

Yes, I think it is possible. I will take a look

@@ -70,4 +71,12 @@ public boolean isPrebuiltTask() {
public int[] getAtomics() {
return atomics;
}

public void withClass(Class<?> klass) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should have been withKlass, right?

this.klass = klass;
}

public Class<?> getClassJar() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

and this getKlass?

Copy link
Member Author

Choose a reason for hiding this comment

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

In Java, there is a method called getClass, so to avoid miss-understanding about the functionality of each, I named it ClassJar

Comment on lines 250 to 256
public Class<?> getClassJAR() {
return klass;
}

public void withKlassJAR(Class<?> classJar) {
this.klass = classJar;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure about the classJar and getClassJAR, withKlassJAR names. To my understanding you pass the name of the class file (e.g. MyKlass.class) and not the jar, right? it is assumed that the jar must be in the class file.

Is this correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

We pass any class that appears in the JAR file that we want to load

@jjfumero
Copy link
Member Author

Test provided:

tornado-test -V uk.ac.manchester.tornado.unittests.prebuilt.PrebuiltTest#testPrebuiltWithJar

Works for OpenCL and PTX. I am checking for SPIR-V.

@jjfumero jjfumero marked this pull request as draft July 25, 2024 14:32
@jjfumero
Copy link
Member Author

jjfumero commented Jul 25, 2024

I reverted this PR to draft again. Since this is very specific to the AERO project, we might only added it in the AERO GitHub space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants