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

Tue Apr 30 10:32:23 PDT 2013

Anyone that depends on strict layering will have their builds break. I'm
cool with Filip's idea though.
On Apr 30, 2013 7:15 PM, "Kaylor, Andrew" <andrew.kaylor at intel.com> wrote:

> So should I commit Filip's patch with the understanding that the wrap
> stuff will be split out later, or should I wait for another revision?
>
> -Andy
>
> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:
> llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Eric Christopher
> Sent: Tuesday, April 30, 2013 4:34 AM
> To: Filip Pizlo
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH][llvm-c] Expose CodeModel and EnableFastISel through
> the MCJIT C API
>
> Yeah, I admit to being awful at naming things. Sounds good. Please commit
> separately or I can do it if you'd like.
>
> -eric
>
> On Tue, Apr 30, 2013 at 4:53 AM, Filip Pizlo <fpizlo at apple.com> wrote:
> > Eric introduced Wrap.h some revisions ago, and I actually don't mind
> > that name. But what CBindingWrapping.h?
> >
> > -Filip
> >
> > On Apr 29, 2013, at 8:38 PM, Evan Cheng <evan.cheng at apple.com> wrote:
> >
> > Hi Filip,
> >
> > Sorry I missed your previously patch. I just have a nitpick about
> > Wrap.h. It seems like an awfully generic name. Is it possible to come
> > up with a more descriptive one?
> >
> > Evan
> >
> > On Apr 29, 2013, at 6:13 PM, Filip Pizlo <fpizlo at apple.com> wrote:
> >
> > Updated patch, which fixes line lengths.
> >
> > <mcjit.patch>
> >
> > -F
> >
> >
> > On Apr 29, 2013, at 5:38 PM, "Kaylor, Andrew"
> > <andrew.kaylor at intel.com>
> > wrote:
> >
> > I don't think I'd say the problems with code models and fast isel are
> > blockers.  You can get into the same problems with the C++ interface.
> > Mostly I wanted to mention it as a first place to look if things go
> > haywire when you try new combinations.  Some combinations don't
> > usually make sense (like small code model on x86-64), but others just
> > fail because we don't have all the necessary relocations implemented.
> > There's no reason for the C-interface to be any more restrictive than
> > the C++ interface in this regard.
> >
> > If you clean up the line lengths this should be ready to go.
> >
> > -Andy
> >
> > -----Original Message-----
> > From: Filip Pizlo [mailto:fpizlo at apple.com]
> > Sent: Monday, April 29, 2013 5:25 PM
> > To: Kaylor, Andrew
> > Cc: llvm-commits at cs.uiuc.edu
> > Subject: Re: [PATCH][llvm-c] Expose CodeModel and EnableFastISel
> > through the MCJIT C API
> >
> >
> >
> > On Apr 29, 2013, at 4:25 PM, "Kaylor, Andrew"
> > <andrew.kaylor at intel.com>
> > wrote:
> >
> > 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?
> >
> >
> > True!  I did because of compactness; I believe that the enum is 4
> > bytes and the bio leans are bytes. I wanted the strict to be well packed.
> >
> >
> > 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.
> >
> >
> > Oh, I'm sorry!  I will make sure I check that before submitting.
> >
> >
> > 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.
> >
> >
> > Thanks for the heads up. Do you think this is a blocker for exposing
> > these options, or just an FYI?
> >
> >
> > -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.
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130430/57590022/attachment.html>