Initial Metal backend support to dcompute - #5118
Conversation
|
In cmakelist, LDC_LLVM_VER was removed. Try LLVM_VERSION_MAJOR. |
be7bad2 to
5906ad4
Compare
thewilsonator
left a comment
thewilsonator
left a comment
There was a problem hiding this comment.
Looking good. Needs some tests.
| } | ||
| }; | ||
|
|
||
| struct DcomputeMetalScalarRewrite : ABIRewrite { |
There was a problem hiding this comment.
it still used in abi/metal.cpp file:
struct MetalABI : TargetABI {
DComputePointerRewrite pointerRewite;
DcomputeMetalScalarRewrite metalScalarRewrite;
.....
It is used to make scalar objects turn into a pointer in constant memory address space. Although it turns out scalars can also be stored in device level address space which will be Global in the dcompute term as I understand
There was a problem hiding this comment.
scalar arguments to kernels are just passed as parameters
| * valid values of _version are for OpenCL 100 110 120 200 210 | ||
| * and for CUDA are x*100 + y*10 for x any valid values of sm x.y | ||
| * use 0 as a wildcard to match any version. | ||
|
|
There was a problem hiding this comment.
ditto, also could you update the documentation here to specify what are the valid versions of Metal (e.g. 400)?
| if (useIR2ObjCache) { | ||
| cache::cacheObjectFile(filename, moduleHash); | ||
| } | ||
| } |
…er and load them in the function body
| @@ -0,0 +1,33 @@ | |||
| // REQUIRES: target_AArch64 | |||
| // REQUIRES: llvm20 | |||
There was a problem hiding this comment.
this should be an "atleast" requirement
There was a problem hiding this comment.
Unfortunately it should be REQUIRES because currently there are outstanding issues with the new changes in llvm versions after llvm20. Therefore current changes work only with llvm20.
There was a problem hiding this comment.
the issue was in newer llvm versions some attributes got changed. Here you can find nocapture got changed to captures. Apple llvm bitcode writer does not know captures attribute's existence, it expects nocapture instead.
There was a problem hiding this comment.
i don't understand. Which LLVM bitcode writer?
Upon changing the attribute, ofcourse the bitcode writer is also updated to work with that?
There was a problem hiding this comment.
I guess you mean that we have to match the test with the available xcrun version on the tester. Can we rewrite the test to not rely on running xcrun? Just use the outputted object file (that is passed to xcrun) instead?
There was a problem hiding this comment.
(we don't need to test xcrun, we need to test our output that goes into xcrun, right?)
There was a problem hiding this comment.
Sorry I have not provided more context on this. In the upstream LLVM there is no official Apple Metal backend target, I reverse engineered the necessary metadata that Apple Metal Shading Language compiler (which is old fork of LLVM with adjustments by Apple) expects for the kernel functions. The pipeline is as follows:
DCompute kernel code -> LLVM IR bitcode with reverse engineered metadata ->
xcrun -sdk macosx metallib .... ->
generates <filename>.metallib binary file
We use that <filename>.metallib binary file to execute kernels at the dcompute host code part as follows:
...
auto program = Program.fromFile(device, "./kernels_metal400_64.metallib");
Program.globalProgram = program;
auto deviceX = device.makeBuffer!float(x);
auto deviceY = device.makeBuffer!float(y);
auto deviceRes = device.makeBuffer!float(res);
auto queue = Queue(device);
queue.enqueue!(saxpy)
([N,1,1],[1,1,1])
(deviceRes, alpha, deviceX, deviceY, N);
queue.finish();
...
The bitcode issue is that xcrun uses Apple toolchain which is closed source LLVM fork. Current changes offer basic support. What I am thinking to do afterwards is to explore to write custom bitcode writer as done in Julia compiler (https://github.com/JuliaLLVM/llvm-downgrade/blob/main/src/BitcodeWriter140.cpp). They mostly copy & pasted older llvm bitcode writer and adjusted a bit for their codegen. They also did similar changes to emit correct metadata as in this PR. (https://github.com/JuliaGPU/GPUCompiler.jl/blob/main/src/metal.jl)
There was a problem hiding this comment.
It's just that this is quite flaky... and this test will start to fail when the system xcode version changes.
There was a problem hiding this comment.
nope, it will not. In the test I only tested what would be generated as LLVM IR from ldc compiler not the output of xcrun.
There was a problem hiding this comment.
all the metadata is added at the ldc side and llvm bitcode is emitted and sent over to xcrun afterwards. Tests in this PR only tested the emitted metadata from ldc side only. But once xcrun -sdk macosx metallib compiler changes what it expects from the Apple Metal Shading language compiler (closed old fork of LLVM), only then we might have to adjust the metal target metadata codegen part in ldc and these tests.
|
|
||
| #ifdef LDC_LLVM_SUPPORTED_TARGET_AArch64 | ||
| if (cb == ComputeBackend::METAL) { | ||
| #ifdef __APPLE__ |
There was a problem hiding this comment.
This ifdef would mean LDC cannot produce metal bitcode output on other platforms, and instead will silently not create any output.
I think this code should be included on any LDC build (it is a cross-compiler after all). xcrun would not be found, perhaps make that a warning instead of a fatal error. Generating the object file could still be useful, no?
| int status = executeToolAndWait(Loc(), args[0], args); | ||
|
|
||
| if (status < 0) { | ||
| error(Loc(), "program received signal %d (%s)", -status, |
There was a problem hiding this comment.
"program" can be very ambiguous here. Why not call it "xcrun" ?
4 participants
No description provided.