Defines functions as static

[koparasy]

No description provided.

[ggeorgakoudis]

I don't see the usefulness of this refactoring.

Why do static for the methods?

Also, RTC usage is covered by the Config option for HIP. Yes, the #ifdef is ugly but will be dropped once we bump up support to LLVM 18.

[koparasy]

The usefulness is for proteus users to be able to call these functions without instantiating a proteus JIT object. The proteus JIT object is always a singleton and is bound to the environment variables and a cache which users may or may not want to use.

These functions do not modify (modulo the current error on cuda) do not modify the instance of the class.

[ggeorgakoudis]

The usefulness is for proteus users to be able to call these functions without instantiating a proteus JIT object. The proteus JIT object is always a singleton and is bound to the environment variables and a cache which users may or may not want to use.

These functions do not modify (modulo the current error on cuda) do not modify the instance of the class.

Is this a blocker for your use-case? You can instantiate the singleton and call from the object. I'm afraid that if we start making methods as static we will increasingly have to pass otherwise object state as arguments. The env variables are also a desirable feature.

What's the concern with calling through the object?

[ggeorgakoudis]

I see you need to pass GlobalLinkedBinaries. Is that because Mneme will independenttly find out those instead of relying on the ProteusPass?

[koparasy]

Yes.

[koparasy]

The usefulness is for proteus users to be able to call these functions without instantiating a proteus JIT object. The proteus JIT object is always a singleton and is bound to the environment variables and a cache which users may or may not want to use.
These functions do not modify (modulo the current error on cuda) do not modify the instance of the class.

Is this a blocker for your use-case? You can instantiate the singleton and call from the object. I'm afraid that if we start making methods as static we will increasingly have to pass otherwise object state as arguments. The env variables are also a desirable feature.

What's the concern with calling through the object?

The question I would first ask is why these methods:

optimizeIR
createTargetMachine
runCleanupPassPipeline
setLaunchBoundsForKernel
setKernelDims
codegenPTX
setLaunchBoundsForKernel

are not static. They are not modifying the object and never access *this.

Now for codegenObject and linkJitModule I can see the point of passing additional arguments. That may not be correct. But in any case for the codegenObject an external user will nver have access to the GlobalLinkedBinaries. As it is likely that there is not going to be another binary in the system as your current use case.

I want to have a generic functionality on proteus that does the following:

KFunc = ToExecutable(LLVMIRModules, KernelName)

and with these modifications I can have a clear path of having my function that does:

JIT(LLVMIRModules, KernelName){
     auto Mod = linkJITModule(LLVMIRModules);
     internalize(Mod, KernelName);
     F = Mod.getFunction(KernelName);
     setKernelDims(Mod);
     setLaunchBoundsForKernel(F)
     cleanup(M)
     optimizeIR(M)
     return codeGenObject(Object)
}

With no caches, with no environment variables. I can see why the proteus project requires to have these, but I don't see why another user needs to always generate directories or why he needs to specify environment variables that are not defined in my (third-party) code. If in the a third party user requires all of these, he is free to declare the singleton and get all of it, but this is not imposed by the API it is a choice of the user. If proteus adds some feature that no longer can work with static (for whatever reason) the user will decide whether to update their code or not.

I am not that worried about passing additional parameters on these functions. As these operate always on the LLVM Module and on the Object. Even if you add functionality that requires access to the instance. The above functions should still exist.

[ggeorgakoudis]

I understand your use-case now better. I suggest next time you explain in the comments the purpose of your PR before asking for review.

You want to use just a subset of codegen/transform aspects of the proteus functionality, which is fine.
Yes, some methods you want to use do not access object state, but others do, as you spot them and make this state an argument instead. My issue with making them static methods is that now we must implicitly assume those methods must not use object state, even for future development, because they're part of an implicit external interface.

As discussed offline, I'd consider that for an interim refactoring stage, but clearly we should refactor again.