Move CodeGen_GPU_Host to a lowering pass

[dsharletg]

Here's what I think we should do on this issue. We can't do what #5567 did to the other targets because CodeGen_GPU_Host needs them as CRTP base classes.

We can fix that by refactoring CodeGen_GPU_Host to be a lowering pass like HexagonOffload.cpp instead. I think this would generally be a great cleanup to do anyways. CodeGen_GPU_Host wouldn't be needed any more. This will be much easier after OpenGL goes away (which is #5626), because OpenGL vertex buffer stuff is the hardest thing to move to a separate lowering pass.

After that is done, we can just do basically the same thing as #5567 to the rest of the CodeGen_* classes.

Originally posted by @dsharletg in #5503 (comment)

[JanHett]

Hello @dsharletg! Sorry for the late reply. To answer your question from #5503, I'd be happy to work on this but I feel like I don't understand the internals of Halide well enough to get started. Code generation et Cie are definitely outside my "comfort zone", but if you can point me at some resources relevant to Halide's implementation I would certainly be interested in giving this refactor a good try.

[dsharletg]

Thanks for offering to take a look. The basic thing that needs to happen is that at some point during compilation, we need to inject calls to the runtime functions to implement GPU tasks. Currently, this happens in two places:

• InjectHostDevBufferCopies.cpp: this adds calls to halide_copy_to_device and halide_copy_to_host as needed.
• CodeGen_GPU_Host.cpp: This inserts calls to halide_*_run, halide_*_initialize_kernels and related runtime functions as needed.

CodeGen_GPU_Host is implemented by directly generating LLVM IR to implement the calls it makes, while InjectHostDevBufferCopies.cpp generates Halide IR first, as part of lowering (in Lower.cpp).

HexagonOffload.cpp is an example of how we should do things instead. Compare HexagonOffload generating calls to halide_hexagon_run with Halide IR:

Halide/src/HexagonOffload.cpp

Line 915 in 1155bda

Stmt offload_call = call_extern_and_assert("halide_hexagon_run", params);

vs. CodeGen_GPU_Host call halide_*_run as LLVM IR:

Halide/src/CodeGen_GPU_Host.cpp

Line 504 in 1155bda

Value *result = builder->CreateCall(dev_run_fn, launch_args);

If we can move all of these generated calls from CodeGen_GPU_Host to a separate lowering pass like HexagonOffload, we could get rid of CodeGen_GPU_Host completely. This would be an improvement for a number of reasons:

• Currently, we can't target GPUs from non-LLVM based code generators. This would fix that.
• It would not need the CodeGen_* base classes of CodeGen_GPU_Host to be visible anywhere. The implementations of these could be moved to their cpp files, and more easily conditionally compiled (the problem you were trying to fix).
• It is just a good general cleanup.

[sakehl]

Hi, @dsharletg responded to me on Halide gitter a few days back.

I've been busy with adding OpenCL to the C/C++ backend (you can now call a function with .compile_to_opencl. I've added it to my fork: sakehl/Halide@d925ffa

I think for this issue, very similar things need to happen as I've done in the visit function (for For loops):
sakehl@d925ffa#diff-593c5627459bfa466cb11461822c0e9ace99be88d8838375451bf1778dc19b62R40

And this compile function:
sakehl@d925ffa#diff-593c5627459bfa466cb11461822c0e9ace99be88d8838375451bf1778dc19b62R209

The big plus is that it is completely separate from the LLVM back-end, so a pass would do mostly the same things, but then use call_extern_and_assert for the correct functions.

A working example of what I implemented is:

#include "Halide.h"
#include <stdio.h>

using namespace Halide;

int main(int argc, char **argv) {
    Func in("in"), even("even");
    Var x("x");

    in(x) = 2*x*x;
    even(x) = in(x) + in(x+1);

    Var bx("bx"), tx("tx");
    in.compute_root();
    in.gpu_tile(x, bx, tx, 32);
    even.gpu_tile(x, bx, tx, 32);
    even.bound(x,0,100);

    Target target = get_host_target();
    Target new_target = target
      .with_feature(Target::OpenCL)
      ;
    assert(host_supports_target_device(new_target));
    compile_standalone_runtime("opencl_runtime.o",new_target);

    even.compile_to_opencl("even_c.cpp", {}, "even", new_target);
    even.compile_to_header("even_c.h", {}, "even", new_target);
}

After which you compile this code, run it, and afterwards compile evenc.c like:
g++ -Wall -o even_c_run -I ../Halide-install/include/ even_c_run.cpp even_c.cpp opencl_runtime.o -lpthread -ldl
Where even_c_run.cpp is a simple file containing:

#include "HalideBuffer.h"
#include "even_c.h"

#include <stdio.h>

int main(int argc, char **argv) {
    int n = 100;
    Halide::Runtime::Buffer<int> output(n);

    int error = even(output);
    output.copy_to_host();

    if (error) {
        printf("Halide returned an error: %d\n", error);
        return -1;
    }

    for (int x = 0; x < n; x++) {
        uint8_t output_val = output(x);
        uint8_t correct_val = 2*x*x + 2*(x+1)*(x+1);
        if (output_val != correct_val) {
            printf("output(%d) was %d instead of %d\n",
                   x, output_val, correct_val);
            return -1;
        }
    }

    // Everything worked!
    printf("Success!\n");
    return 0;
}

[steven-johnson]

Re: #5757, that looks interested and definitely related. Being able to see the host code in C++ form would indeed be a useful thing (and as I mentioned over there, I wouldn't be surprised if the code generated by that approach would be comparable in performance to what LLVM generates, since it's generally just short snippets of loopless setup code).

[sakehl]

I've given it some more thought, and this is probably the best way to go forward. It should be very similar to what was already done in my changes, but move it to a lowering pass. What I wrote there is very easy to generalize to other GPU backends, but I focussed on getting OpenCL working first. Maybe I'll give it a try when I have some free time this weekend, I'll keep you updated.

[dsharletg]

@sakehl sorry if you already started this, but I decided to try this yesterday after so many people ran into this issue: https://github.com/halide/Halide/compare/dsharletg/offload-gpu?expand=1

I was just waiting on the buildbots to come back up before opening a PR from that branch.

[sakehl]

@dsharletg, ah no I didn't start yet, and your PR looks great, thanks!