llvm dev - Feb 2013 - [LLVMdev] GCMetadataPrinter::finishAssembly not executed?

On 02/15/2013 07:13 PM, Pedro Artigas wrote:
> I am not an expert on metadata or the CG information but one of the two has
to happen:
I'm not an expert either but I'll give it a try! :-)
> - Simply moving the deleter pass to a different spot
> - Changing the doFinalization of another pass (Printer?) to do the deleter
pass job (this should work now because the doFinalization order is the reverse
of the doInitialization order)
> 
> Option 1 is a smaller change, option 2 will make the code cleaner and
prevent issues like this in the future, so it sounds like a better solution.
I'd vote for the second one! Does this mean we don't want GCInfoDeleter
any more? Because, IMO, we could do that by just moving the
Deleter::doFinalization functionality to Printer::doFinalization
(introduce it as this doesn't exist).

The attached patch seems to work. Take it as a prototype as it needs a
unittest just to be sure in the future!

Does this seem reasonable?

yianns

-- 
Yiannis Tsiouris
Ph.D. student,
Software Engineering Laboratory,
National Technical University of Athens
WWW: http://www.softlab.ntua.gr/~gtsiour
-------------- next part --------------
A non-text attachment was scrubbed...
Name: GCMetadata.patch
Type: text/x-diff
Size: 2199 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130216/0609b1f0/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130216/0609b1f0/attachment.sig>

Hello Yiannis,

Your patch seems fine. It is option #2 which I am glad to hear solves the issue.
It also removes more lines than it adds, I like patches that fix issues and have
that property!

Anyhow if you add some testing, as you suggest, you should be good to go.

Thanks

Pedro

On Feb 16, 2013, at 6:13 AM, Yiannis Tsiouris <gtsiour at softlab.ntua.gr>
wrote:
> <GCMetadata.patch>

Pedro, Yiannis,

What's about the usage case, when LLVM is used as a library and the user
implements its custom pass, which dump the code (implemented as a
FunctionPass, but not as Printer)?

You also missed in your changes the declaration
of llvm::createGCInfoDeleter() in include/llvm/CodeGen/Passes.h

-Dmitry.

On Mon, Feb 18, 2013 at 9:34 PM, Pedro Artigas <partigas at apple.com>
wrote:
>
> Hello Yiannis,
>
> Your patch seems fine. It is option #2 which I am glad to hear solves the
> issue. It also removes more lines than it adds, I like patches that fix
> issues and have that property!
>
> Anyhow if you add some testing, as you suggest, you should be good to go.
>
> Thanks
>
> Pedro
>
> On Feb 16, 2013, at 6:13 AM, Yiannis Tsiouris <gtsiour at
softlab.ntua.gr>
> wrote:
>
> > <GCMetadata.patch>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130223/7d51b8cd/attachment.html>