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
part-1.patch (3.53 KB)
part-2.patch (10.2 KB)
part-3.patch (7.74 KB)
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
thanks for the response,
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.
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.
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@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?
thanks for the response,
> 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@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
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.
Is their any problem when a patch is committed, then gets pulled back into your git fork? Does it treat the changes coming from the svn mirror as conflicts?
If it matches exactly, git ignores it. If it's different, you can
either merge it (which is easy in git), or just skip it.
- Michael Spencer