[LLVMdev] MC-JIT

[Olivier Meurant]

New patch taking Eli's comments into account.

Olivier.

On Tue, Jul 20, 2010 at 11:09 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Tue, Jul 20, 2010 at 1:36 PM, Olivier Meurant
> <meurant.olivier at gmail.com> wrote:
>>> Seems reasonable, but I haven't looked at the code yet. I would
>>> suggest trying to split your work up into separate patches which
>>> implement incremental pieces of functionality, then submitting them to
>>> llvm-commits for review. That is much easier for us to deal with than
>>> large monolithic patches out of tree.
>>
>> Ok. Sorry for the too big patch. Attached is the first patch adding
>> only 2 hooks on TargetMachine and on MCAssembler. Style should be LLVM
>> compliant. Apply it with "patch -p0".
>
> +  // Make sure the code model is set.
> +  setCodeModelForStatic();
>
> For the JIT, this should be setCodeModelForJIT; it makes a difference
> on, for example, x86-64.  I'd also suggest changing the name of the
> method appropriately.
>
> -  llvm::OwningPtr<MCObjectWriter> Writer(getBackend().createObjectWriter(OS));
> -  if (!Writer)
> -    report_fatal_error("unable to create object writer!");
> +  MCObjectWriter *Writer = Writer_;
> +  if (Writer == 0) {
> +      Writer = getBackend().createObjectWriter(OS);
> +      if (!Writer)
> +          report_fatal_error("unable to create object writer!");
> +  }
>
> It would be cleaner if you kept around the OwningPtr, but only
> assigned to it in the case where the writer take ownership; that way,
> you wouldn't have to mess with explicitly deleting the writer.  Also,
> the surrounding code uses two spaces for indentation, not four.
>
> I think it looks fine otherwise.
>
> -Eli
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mc_jit_01.patch
Type: text/x-patch
Size: 4411 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100721/87147d95/attachment.bin>