[llvm-commits] Adding a builder for ExecutionEngines

Evan Cheng evan.cheng at apple.com
Mon Jul 13 22:15:51 PDT 2009


Thanks Reid. Sorry about the late review. I just have a few nitpicks  
in addition to what Jeffrey has pointed out.

+ public:
+  enum EnginePreference {
+    EITHER,
+    INTERP,
+    JITONLY
+  };

LLVM is a polite compiler. We don't do all-caps. :-) How about  
InterpOnly, JITOnly as bitmasks? Then "EITHER" would just be the two  
or'red together?

+  /// init - Does the common initialization of default options.
+  ///
+  void init() {

I prefer a more explicit name. Perhaps InitEngine()?

Thanks,

Evan

On Jul 8, 2009, at 4:35 PM, Reid Kleckner wrote:

> In the last patch I was working on, Evan asked if I could add a
> builder for ExecutionEngines, since the current API call is kind of
> unwieldy.  It has four optional arguments, and a fifth if you count
> the JIT memory manager.  I attempted to unify this all into just one
> EngineBuilder class, which makes it so you only have to specify the
> arguments you want to control with chained setter methods.
>
> Does this seem like a better API?  Please review.
>
> Patch attached and linked:
> http://codereview.appspot.com/91086/show
>
> Thanks,
> Reid
> <EngineBuilderPatch.diff>




More information about the llvm-commits mailing list