Write JitBuilder C API generator

[Leonardo2718]

As a step towards addressing #2049, the following work needs to be completed:

• [ ] Refactor C++ generator so code can be shared more easily with C generator
  • [x] Create a "generator" class
  • [ ] Create a source file writer class
  • [ ] Common "generator driving" code
• [ ] Migrate C++ JitBuilder examples to C
• [ ] Write generator for C
  • [ ] Generate header files with forward declarations
  • [ ] Generate source files with implementation

[daytonjallen]

Hey, just to get the discussion started on what the C API implementation should look like, I've gone ahead and translated the Simple.cpp & Simple.hpp examples from the test suite to C as a proposal of how the API might look.

Simple.h

#ifndef SIMPLE_H
#define SIMPLE_H

#include "Jitbuilder.h"

typedef struct SimpleMethod {
    /* Struct embedding for inheritance? */
    MethodBuilder *methodBuilder;
} SimpleMethod;

SimpleMethod* SimpleMethod_init(TypeDictionary *);
bool SimpleMethod_buildIL(SimpleMethod *);

#endif SIMPLE_H

Simple.c

#include "Simple.h"

#include <stdio.h>
#include <stdbool.h>
#include <stdlib.h>

#define TOSTR(x) #x
#define LINETOSTR(x) TOSTR(x)

int main(int argc, char *argv[]) {
    puts("Step 1: initialize JIT");
    bool initialized = JB_initializeJit();
    if (!initialized) {
        fprintf(stderr, "FAIL: could not initialized JIT\n");
        exit(-1);
    }

    puts("Step 2: define type dictionary");
    TypeDictionary *types;
    puts("Step 3: compile method builder");
    SimpleMethod *method = SimpleMethod_init(types);

    void *entry = 0;
    int32_t rc = JB_compileMethodBuilder(method->methodBuilder, &entry);
    if (rc != 0) {
        fprintf(stderr, "FAIL: compilation error %d\n", rc);
        exit(-2);
    }

    puts("Step 4: invoke compiled code and print results\n");
    typedef int32_t (SimpleMethodFunction)(int32_t);
    SimpleMethodFunction *increment = (SimpleMethodFunction *) entry;

    int32_t v;
    v = 0;
    printf("increment(%d) == %d\n", v, increment(v));

    v = 1;
    printf("increment(%d) == %d\n", v, increment(v));

    v = 10;
    printf("increment(%d) == %d\n", v, increment(v));

    v = 15;
    printf("increment(%d) == %d\n", v, increment(v));

    puts("Step 5: shutdown JIT");
    JB_shutdownJit();
}

SimpleMethod* SimpleMethod_init(TypeDictionary *td) {
    SimpleMethod* sm = (SimpleMethod*)malloc(sizeof(SimpleMethod));
    sm->methodBuilder = MethodBuilder_init(td, (VirtualMachineState*) NULL);

    /* Emulate virtual methods using a function pointer? */
    sm->methodBuilder->ilBuilder->buildIL = SimpleMethod_buildIL;

    MB_DefineLine(sm->methodBuilder->, LINETOSTR(__LINE__));
    MB_DefineFile(sm->methodBuilder->, __FILE__);
    MB_DefineParameter(sm->methodBuilder->, "value", td->Int32);
    MB_DefineReturnType(sm->methodBuilder->, td->Int32);
}

MethodBuilder* MethodBuilder_init(TypeDictionary *td, VirtualMachineState *vms) {
    MethodBuilder* mb = (MethodBuilder *) malloc(sizeof(MethodBuilder));
    mb->ilBuilder = IlBuilder_init();
    return mb;
}

bool SimpleMethod_buildIL(SimpleMethod* simpleMethod) {
    puts("SimpleMethod::buildIL() running!");
    ILBuilder* ilBuilder = simpleMethod->methodBuilder->ilBuilder;
    IB_Return(ilBuilder,
            IB_Add(ilBuilder, 
                    IB_Load(ilBuilder,"value"),
                    IB_ConstInt32(ilBuilder, 1))
    );
    return true;
}

[Leonardo2718]

@daytonallen I think this is headed in the right direction. A few thoughts:

• Traditionally, we haven't used _ in C++ method names (except in a few rare circumstances). It might make the names more readable in this case, but we should see what others think.

  /* Emulate virtual methods using a function pointer? */
  sm->methodBuilder->ilBuilder->buildIL = SimpleMethod_buildIL;

• Yes, callbacks will be set using function pointers. However, just setting a field will not be enough as the callback pointer on the implementation side will also have to be updated. Instead, there should be an API function that takes care of this.

  ILBuilder* ilBuilder = simpleMethod->methodBuilder->ilBuilder;

• I'm not a fan of using the parent class name as field name for the "parent pointer" because it might get confusing in more complex examples where multiple IlBuilder objects are involved. Instead, I was thinking of using the name super for all parent pointers.
• The examples should demonstrate how the JitBuilder C API is intended to be used, so we should have a discussion to decide this now. For example, strictly speaking, users don't need to create a struct that "inherits" for MethodBuilder, they only need to provide the buildIL() callback. However, not using a custom struct could make managing state more difficult.

@mstoodle any thoughts?

[mstoodle]

I don't think this part works for me:

    TypeDictionary *types;
    puts("Step 3: compile method builder");
    SimpleMethod *method = SimpleMethod_init(types);

I expected more of a TypeDictionary *types = TD_NewDictionary(); or something like that. Then I'm not sure you need a "SimpleMethod" struct in this scenario, as @Leonardo2718 pointed out. I think you'd just do MethodBuilder *sm = MB_NewMethod(types); or even MethodBuilder sm; MB_Initialize(&sm);

[mstoodle]

That MB_Initialize() call could even take the buildIL callback pointer.

Then the rest of the constructor can just be executed inline as a set of calls on mb (or collected together in an init() function as @daytonallen did).

[mstoodle]

@Leonardo2718 said "Instead, there should be an API function that takes care of this." on the setting of the buildIL callback pointer. It can be passed as a constructor argument, as I mentioned and primarily as a form of convenience. But it should also be changeable, and I agree that should definitely be recommended to be done via an API call rather than by reaching in to set the field directly.

[mstoodle]

Thanks for starting to push this one forward, @daytonallen and @Leonardo2718 . A working and flexible C API paves the way for other language bindings, and I'm very excited to see forward progress here!

[Leonardo2718]

FYI: I've opened #3289, which does most of the refactoring I had planned in preparation for the C API generator work. I'm hoping to open a follow up PR with some initial work on the C generator very soon.

[daytonjallen]

Hey, sorry for the delayed response.

• As an alternate naming convention, we could use full camelcased names, so MethodBuilderAdd instead of MB_Add.
• Regarding the parent pointer name issue @Leonardo2718, I'm guessing you meant changing this snippet

  ILBuilder* ilBuilder = simpleMethod->methodBuilder->ilBuilder;
  IB_Return(ilBuilder,
          IB_Add(ilBuilder, 
                  IB_Load(ilBuilder,"value"),
                  IB_ConstInt32(ilBuilder, 1))
  );

  To something more like

  ILBuilder* super = simpleMethod->methodBuilder->ilBuilder;
  IB_Return(super,
          IB_Add(super, 
                  IB_Load(super,"value"),
                  IB_ConstInt32(super, 1))
  );

• The TypeDictionary issue was an oversight on my part, I forgot the implicit default constructor call from the cpp code. It would indeed be something more like what you suggested @mstoodle .
• Regarding buildIL, if I understand clearly
  • Have an API call

    setBuildILCallback(IlBuilder* ilBuilder, bool (*buildIL)(void));
    /* Not sure how the inner mechanics of this would work to update the pointer on the implementation side*/

  • And as a convenience within the MethodBuilder constructor

    MethodBuilder* MethodBuilder_init(TypeDictionary *td, VirtualMachineState *vms, bool (*buildIL)(void) ) {
    MethodBuilder* mb = (MethodBuilder *) malloc(sizeof(MethodBuilder));
    mb->ilBuilder = IlBuilder_init();
    if (buildIL) {
        ILBuilder* super = mb->ilBuilder;
        setBuildILCallback(super, buildIL);
    }
    return mb;
    }

    After clearing up everything, then I'll iterate on the initial suggestions and repost a concrete proposal.

[Leonardo2718]

@daytonallen

• I think using the full class name is a bit verbose, I prefer the short prefix (which is already specified in the API description)
• Actually what I meant was:

ILBuilder* ilBuilder = simpleMethod->super->super;

• Yes, following the style of the C++ API, we'll need something like:

void setBuildILCallback(IlBuilder* self, bool (*buildIL)(void)) {
  static_cast<TR::IlBuilder *>(self->impl_)->setClientCallback_buildIL(static_cast<void*>(buildIL));
}

[daytonjallen]

Gotcha, thanks. Where can I find the API description that you're referencing?

[Leonardo2718]

The API description is in the directory containing the C++ generator: omr/jitbuilder/apigen/jitbuilder.api.json.

[daytonjallen]

Hey, this is the second iteration with all the proposed changes thus far:

Simple.c

#include <stdio.h>
#include <stdbool.h>
#include <stdlib.h>

#include "Jitbuilder.h"

#define TOSTR(x) #x
#define LINETOSTR(x) TOSTR(x)

int main(int argc, char *argv[]) {
    puts("Step 1: initialize JIT");
    bool initialized = JBinitializeJit();
    if (!initialized) {
        fprintf(stderr, "FAIL: could not initialized JIT\n");
        exit(-1);
    }

    puts("Step 2: define type dictionary");
    TypeDictionary *types = TDInit();
    puts("Step 3: compile method builder");
    MethodBuilder *method = MBInit(types, (VirtualMachineState*) NULL, buildIL);

    MBDefineLine(method, LINETOSTR(__LINE__));
    MBDefineFile(method, __FILE__);
    MBDefineParameter(method, "value", types->Int32);
    MBDefineReturnType(method, types->Int32);

    void *entry = 0;
    int32_t rc = JBcompileMethodBuilder(method, &entry);
    if (rc != 0) {
        fprintf(stderr, "FAIL: compilation error %d\n", rc);
        exit(-2);
    }

    puts("Step 4: invoke compiled code and print results\n");
    typedef int32_t (SimpleMethodFunction)(int32_t);
    SimpleMethodFunction *increment = (SimpleMethodFunction *) entry;

    int32_t v;
    v = 0;
    printf("increment(%d) == %d\n", v, increment(v));

    v = 1;
    printf("increment(%d) == %d\n", v, increment(v));

    v = 10;
    printf("increment(%d) == %d\n", v, increment(v));

    v = 15;
    printf("increment(%d) == %d\n", v, increment(v));

    puts("Step 5: shutdown JIT");
    JBshutdownJit();
}

bool buildIL(MethodBuilder* mb) {
    puts("SimpleMethod::buildIL() running!");
    ILBuilder* ilBuilder = mb->super;
    IBReturn(ilBuilder,
              IBAdd(ilBuilder,
                     IBLoad(ilBuilder,"value"),
                     IBConstInt32(ilBuilder, 1))
    );
    return true;
}

The MBInit() would look something like

MethodBuilder* MBInit(TypeDictionary *td, VirtualMachineState *vms, bool (*buildIL)(void)) {
    MethodBuilder* mb = (MethodBuilder *) malloc(sizeof(MethodBuilder));
    mb->super = IBInit();
    if (buildIL) {
        ILBuilder* ilBuilder = mb->super;
        setBuildILCallback(ilBuilder, buildIL);
    }
    return mb;
}

[daytonjallen]

Hey @Leonardo2718, what would be the next step after we settle on an API design?

[Leonardo2718]

@daytonallen

I'd say open a pull request adding the C Simple example to jitbuilder/release/c/samples. It will be easier to discuss details that way.