thanks for the response,<br><br><div class="gmail_quote">On Wed, Oct 6, 2010 at 1:53 PM, Michael Spencer <span dir="ltr"><<a href="mailto:bigcheesegs@gmail.com">bigcheesegs@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">On Sun, Sep 26, 2010 at 4:02 PM, Nathan Jeffords<br>
<<a href="mailto:blunted2night@gmail.com">blunted2night@gmail.com</a>> wrote:<br>
> Hi guys,<br>
> Attached is a set of patches that adds detailed argument information to the<br>
> output of the MCLoggingStreamer that may be enabled when outputting an<br>
> object file from llc or llvm-mc. It is broken into three pieces.<br>
> The first patch allows a MCSection to report a name to be used for<br>
> diagnostic purposes.<br>
<br>
</div>I'm not sure if it may be a better idea to just make getSectionName<br>
virtual in the base class instead of adding a new getName function<br>
that just forwards to getSectionName in each subclass. Although<br>
getName is more consistent with the rest of the MC api.<br>
<br>
I propose that we make getSectionName virtual in the base class and<br>
then rename it to getName as it's more consistent and we already know<br>
it is a section because of its type.<br></blockquote><div><br></div><div>From what I can see, getSectionName is only implemented on MachO and it doesn't report the full name.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

<div class="im"><br>
> The second patch updates MCLoggingStreamer to enhance its output.<br>
<br>
</div>LGTM.<br>
<div class="im"><br>
> This third patch enhances the output for instruction in<br>
> the MCLoggingStreamer.<br>
<br>
</div>I think OwningPtr would fix the problems here. Just have the logging<br>
streamer be responsible for deleting the printer.<br>
<br></blockquote><div><br></div><div>As the patch stands, if the output type is assembler, the instruction pointer is created for use with the assembly streamer which I presume takes responsibility for destroying it, otherwise it get created solely for the streamer. Perhaps the cleanest approach would be to instantiate a separate instruction printer just for the logging streamer so their is no confusion as to who's responsibility it is to clean it up.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
- Michael Spencer<br>
<div class="im"><br>
> The first two patches are ready to go in my opinion. But the third doesn't<br>
> deal with ownership of the instruction printer correctly I think, and it<br>
> maybe too much for what it gives back.<br>
> -Nathan<br>
</div>> _______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a>         <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
><br>
><br>
</blockquote></div><br><div>On another note, do you have any advise on staging multiple patches? When generating this set of patches, I had to have three separate copies of code, then using WinMerge, pull the changes for a patch into an intermediate, from their I diffed it against the previous version and generated a patch, then apply the patch to the previous version and repeated the process for each successive patch.</div>
<div><br></div><div>I was looking into using git somehow to create patches for commits, does that seem reasonable?</div><div><br></div><div>- Nathan</div>