[LLVMdev] MCLoggingStreamer enhancements

[Michael Spencer]

On Fri, Oct 8, 2010 at 1:07 AM, Nathan Jeffords <blunted2night at gmail.com> wrote:
> 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 it
> doesn't report the full name.

I see it on all 3. Although on MachO it seems it is different.

>>
>> > 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.

Ah, it seems I completely ignored that InstPrinter was used other
places. The logging streamer does indeed need its own copy.

>>
>> - 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

I use git exclusively. It makes keeping track of separate patch sets
trivial. Just make a branch for each patch set, and have as many
commits in each branch as you desire. To upstream them just do "git
svn rebase". You can also merge in other branches. To export patches
you can just use format-patch which will make a .patch file for each
commit in the branch, although I think you have to remove the headers
to get coreutils-patch to apply it.

- Michael Spencer