Hi guys, Attached is a set of patches that adds detailed argument information to the output of the MCLoggingStreamer that may be enabled when outputting an object file from llc or llvm-mc. It is broken into three pieces. The first patch allows a MCSection to report a name to be used for diagnostic purposes. The second patch updates MCLoggingStreamer to enhance its output. This third patch enhances the output for instruction in the MCLoggingStreamer. The first two patches are ready to go in my opinion. But the third doesn't deal with ownership of the instruction printer correctly I think, and it maybe too much for what it gives back. -Nathan -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100926/2016c78d/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: part-1.patch Type: application/octet-stream Size: 3611 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100926/2016c78d/attachment.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: part-2.patch Type: application/octet-stream Size: 10461 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100926/2016c78d/attachment-0001.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: part-3.patch Type: application/octet-stream Size: 7926 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100926/2016c78d/attachment-0002.obj>
On Sun, Sep 26, 2010 at 4:02 PM, Nathan Jeffords <blunted2night at gmail.com> wrote:> Hi guys, > Attached is a set of patches that adds detailed argument information to the > output of the MCLoggingStreamer that may be enabled when outputting an > object file from llc or llvm-mc. It is broken into three pieces. > The first patch allows a MCSection to report a name to be used for > diagnostic purposes.I'm not sure if it may be a better idea to just make getSectionName virtual in the base class instead of adding a new getName function that just forwards to getSectionName in each subclass. Although getName is more consistent with the rest of the MC api. I propose that we make getSectionName virtual in the base class and then rename it to getName as it's more consistent and we already know it is a section because of its type.> The second patch updates MCLoggingStreamer to enhance its output.LGTM.> This third patch enhances the output for instruction in > the MCLoggingStreamer.I think OwningPtr would fix the problems here. Just have the logging streamer be responsible for deleting the printer. - Michael Spencer> The first two patches are ready to go in my opinion. But the third doesn't > deal with ownership of the instruction printer correctly I think, and it > maybe too much for what it gives back. > -Nathan > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >
thanks for the response, On Wed, Oct 6, 2010 at 1:53 PM, Michael Spencer <bigcheesegs at gmail.com>wrote:> On Sun, Sep 26, 2010 at 4:02 PM, Nathan Jeffords > <blunted2night at gmail.com> wrote: > > Hi guys, > > Attached is a set of patches that adds detailed argument information to > the > > output of the MCLoggingStreamer that may be enabled when outputting an > > object file from llc or llvm-mc. It is broken into three pieces. > > The first patch allows a MCSection to report a name to be used for > > diagnostic purposes. > > I'm not sure if it may be a better idea to just make getSectionName > virtual in the base class instead of adding a new getName function > that just forwards to getSectionName in each subclass. Although > getName is more consistent with the rest of the MC api. > > I propose that we make getSectionName virtual in the base class and > then rename it to getName as it's more consistent and we already know > it is a section because of its type. >>From what I can see, getSectionName is only implemented on MachO and itdoesn't report the full name.> > > The second patch updates MCLoggingStreamer to enhance its output. > > LGTM. > > > This third patch enhances the output for instruction in > > the MCLoggingStreamer. > > I think OwningPtr would fix the problems here. Just have the logging > streamer be responsible for deleting the printer. > >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.> - Michael Spencer > > > The first two patches are ready to go in my opinion. But the third > doesn't > > deal with ownership of the instruction printer correctly I think, and it > > maybe too much for what it gives back. > > -Nathan > > _______________________________________________ > > LLVM Developers mailing list > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > > >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. I was looking into using git somehow to create patches for commits, does that seem reasonable? - Nathan -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20101007/3b24d2f5/attachment.html>