llvm dev - Aug 2010 - [LLVMdev] MC-JIT Patches 2/3

This patch contains the initial implementation of MCJIT. 

- Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0019_mcjit.patch
Type: text/x-diff
Size: 42198 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20100728/2eb6ac65/attachment.patch>

Hi Jan,

I would rather not work with a patch this large. Can you pull out the
addition of the MCJITStreamer into its own patch, and we can iterate
on getting that in as a single commit? I realize it won't work or do
anything useful, but I can't deal with reviewing patches this large.
The main thing I am concerned about is getting the basic design of how
the streamer and the assembler and the object writer should interact.

I also have a few general style comments:
 1. Please use:
--
case A: {
  ...
}
--
instead of:
--
case A:
  {
    ...
  }
--

 2. Please use complete sentences in comments, including leading
capitalization and punctuation. Also, please put a space after the
comment marker ('/// Class', not '///class').

 3. Use doxygen comments when appropriate (for example, on methods,
instance variables).

I have various comments on MCJITStreamer itself, but I will hold them
until we have an individual patch for that class.

 - Daniel

On Wed, Jul 28, 2010 at 10:41 AM, Jan Sjodin <jan_sjodin at yahoo.com>
wrote:
> This patch contains the initial implementation of MCJIT.
>
> - Jan
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>

Okay, I will work on this. It will more or less be one patch per class in this 
case.
Thank you for your commments!

- Jan



----- Original Message ----
> From: Daniel Dunbar <daniel at zuster.org>
> To: Jan Sjodin <jan_sjodin at yahoo.com>
> Cc: LLVM Developers Mailing List <llvmdev at cs.uiuc.edu>
> Sent: Sun, August 1, 2010 7:58:09 PM
> Subject: Re: [LLVMdev] MC-JIT Patches 2/3
> 
> Hi Jan,
> 
> I would rather not work with a patch this large. Can you pull out the
> addition of the MCJITStreamer into its own patch, and we can iterate
> on getting that in as a single commit? I realize it won't work or do
> anything useful, but I can't deal with reviewing patches this large.
> The main thing I am concerned about is getting the basic design of how
> the streamer and the assembler and the object writer should interact.
> 
> I also have a few general style comments:
> 1. Please use:
> --
> case A: {
>   ...
> }
> --
> instead of:
> --
> case A:
>   {
>     ...
>   }
> --
> 
> 2. Please use complete sentences in comments, including leading
> capitalization and punctuation. Also, please put a space after the
> comment marker ('/// Class', not '///class').
> 
> 3. Use doxygen comments when appropriate (for example, on methods,
> instance variables).
> 
> I have various comments on MCJITStreamer itself, but I will hold them
> until we have an individual patch for that class.
> 
> - Daniel
> 
> On Wed, Jul 28, 2010 at 10:41 AM, Jan Sjodin <jan_sjodin at
yahoo.com> wrote:
> > This patch contains the initial implementation of MCJIT.
> >
> > - Jan
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> >
> >
>