[PATCH][llvm-c] Expose CodeModel and EnableFastISel through the MCJIT C API

[Kaylor, Andrew]

Inserting the new CodeModel member of the LLVMMCJITCompilerOptions into the middle of the previous definition would cause problem for anyone using the old structure.  Obviously there hasn't been time for that to be a big deal, but is there a reason you did that?

The way you changed initializing options seems pretty reasonable.

Several lines in your patch go over 80 characters.  At least one line in your previous patch did that too, but I missed it.  LLVM coding standards are against that.

You should be aware that not all code models work well with MCJIT.  We've also had some problems with the fast instruction selectors.  Neither of these things is intentional so much as a current pitfall to be aware of.

-Andy

-----Original Message-----
From: Filip Pizlo [mailto:fpizlo at apple.com] 
Sent: Monday, April 29, 2013 3:35 PM
To: llvm-commits at cs.uiuc.edu
Cc: Kaylor, Andrew
Subject: [PATCH][llvm-c] Expose CodeModel and EnableFastISel through the MCJIT C API

There are more options that are available through the C++ API but not the C API; this patch is an incremental attempt to expose more of those options.  It exposes:

CodeModel: It's now possible to create an MCJIT instance with any CodeModel you like.  Previously it was only possible to create an MCJIT that used CodeModel::JITDefault.

EnableFastISel: It's now possible to turn on the fast instruction selector.

The CodeModel option required some trickery, and I hope that the balance I struck is sensible.  The problem is that previously, we were ensuring future binary compatibility in the MCJITCompilerOptions by mandating that the user bzero's the options struct and passes the sizeof() that he saw; the bindings then bzero the remaining bits.  This works great but assumes that the bitwise zero equivalent of any field is a sensible default value.

But this is not the case for LLVMCodeModel, or its internal equivalent, llvm::CodeModel::Model.  In both of those, the default for a JIT is CodeModel::JITDefault (or LLVMCodeModelJITDefault), which is not bitwise zero.

So I changed this aspect of the API.  Instead of requiring the user to bzero the options struct, I'm now requiring that the user calls LLVMInitializeMCJITCompilerOptions() to initialize the options to the defaults.  The idiom, as shown in the C API Test, looks like:

  LLVMMCJITCompilerOptions options;
  LLVMInitializeMCJITCompilerOptions(&options, sizeof(options));
  options.OptLevel = 2;
  options.NoFramePointerElim = false; // Just ensure that this field still exists.
  
  LLVMExecutionEngineRef engine;
  ASSERT_EQ(
    0, LLVMCreateMCJITCompilerForModule(&engine, module, &options, sizeof(options),
                                        &error));

This still ensures binary future binary compatibility, but lifts the restriction that bitwise zero must be a sensible default.  Binary and source compatibility is ensured with this idiom because:

New LLVM version at build time: if client code was written against an old version of LLVM, but was then built against a newer version, then the user's code would evaluate sizeof(options) differently.  Though the user was unaware of the new fields, LLVMInitializeMCJITCompilerOptions() would know to initialize those fields.

New LLVM version at run time: if client code was built against an old version of LLVM, then it would have an old value of sizeof(options).  LLVMInitializeMCJITCompilerOptions() would only initialize those fields that were within the client's value of sizeof(options).  Then, LLVMCreateMCJITCompilerForModule() would fill in the remaining fields.

This patch also provides common helpers for wrapping and unwrapping CodeModel.  Previously this was done directly in TargetMachineC.cpp, but now we also need it in ExecutionEngineBindings.cpp.  Note that wrap(CodeModel) is currently unused; I don't know what the policy on dead code is; I went with the assumption that erring on the side of completeness (i.e. having symmetry in available wrap/unwrap functions) is the more sensible choice in this particular case.

Finally, this patch *does* break binary compatibility with earlier LLVM revisions that used a different API for creating the MCJIT.  I'm assuming that this is fine since none of this has been officially released yet.  I'm hopeful that the protocol for options is now robust enough that we won't have to mess with it again.