[PATCH] New JIT APIs (ORC)

[Lang Hames]

Hi Justin,

The constructors are templated, but the member hardcodes 4096 so the
> only valid argument to the templates is 4096. Either template the type
> or un-template the constructors?
>

Huh. I had missed that we couldn't even move-construct from other
SmallVectors where the size differed.

I've submitted a patch to llvm-commits to add the necessary operations to
SmallVector so that I can just pass a SmallVectorImpl<T>&&.



> > +  PersistentMangler(DataLayout DL) : DL(std::move(DL)), M(&this->DL) {}
>
> Shouldn't DL be an rvalue reference here?
>

I usually just use pass-by-value where I can. You do end up doubling the
number of move operations for clients who want to move, but that should
still be cheap.


> > +++ include/llvm/ExecutionEngine/Orc/LazyEmittingLayer.h      (working
> copy)
>  ...
> > +    // Build the MangledNames map. Bails out early (with MangledNames
> set to
> > +    // nullptr) if the given SearchName is found while building the map.
> > +    void buildMangledNames(StringRef SearchName,
>
> This actually leaves MangledNames untouched if it bails out early. I
> don't know that it matters much which behaviour we choose, but please
> make the comment and the code match.
>

Comment fixed. Thanks!


> > +++ lib/ExecutionEngine/Orc/IndirectionUtils.cpp      (working copy)
>  ...
> > +std::vector<std::unique_ptr<Module>>
> > +explode(const Module &OrigMod, const JITIndirections &Indirections) {
> > +  std::set<std::string> ImplNames;
> > +
> > +  for (const auto &FuncName : Indirections.IndirectedNames)
> > +    ImplNames.insert(Indirections.GetImplName(FuncName));
> > +
> > +  return explode(
> > +      OrigMod, [&](const Function &F) { return
> ImplNames.count(F.getName()); });
>
> It's better to sort a vector and binary search here, rather than use a
> std::set.
>

Good point. I'm going to leave this as is for now though: explode is
probably going to get completely rewritten soon.

Thanks for the review Justin!


- Lang.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150122/bdcb91ca/attachment.html>