llvm dev - Feb 2013 - [LLVMdev] [PATCH] Fix GCMetadaPrinter::finishAssembly not executed

As discussed in LLVMdev [1], due to the execution order of
doFinalization functions, the GC information were deleted before
AsmPrinter::doFinalization was executed. Thus, the
GCMetadataPrinter::finishAssembly was never called.

The attached patch fixes that by moving the code of the
GCInfoDeleter::doFinalization to Printer::doFinalization. Pedro (cc'ed)
has already taken a look at this and he also thinks that it is OK.

I also tried to add a simple unit-test by using the available Ocaml
garbage collector. I 'm not very familiar with the code of that
collector so I'm not 100% sure that this suffices as a test; for
example, I only test that for the X86-64 architecture, should I add
similar tests for all architectures?

If nobody has any objections, can someone commit this for me? Because
every GC should be unusable right now! :-)


Thanks,
yiannis

[1]: http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-February/059335.html

-- 
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: fix-gcmetadata.patch
Type: text/x-patch
Size: 3302 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130219/355ffc9d/attachment.bin>

On 19.02.2013, at 12:19, Yiannis Tsiouris <gtsiour at softlab.ntua.gr>
wrote:
> As discussed in LLVMdev [1], due to the execution order of
> doFinalization functions, the GC information were deleted before
> AsmPrinter::doFinalization was executed. Thus, the
> GCMetadataPrinter::finishAssembly was never called.
> 
> The attached patch fixes that by moving the code of the
> GCInfoDeleter::doFinalization to Printer::doFinalization. Pedro (cc'ed)
> has already taken a look at this and he also thinks that it is OK.
> 
> I also tried to add a simple unit-test by using the available Ocaml
> garbage collector. I 'm not very familiar with the code of that
> collector so I'm not 100% sure that this suffices as a test; for
> example, I only test that for the X86-64 architecture, should I add
> similar tests for all architectures?
> 
> If nobody has any objections, can someone commit this for me? Because
> every GC should be unusable right now! :-)
Looks great, committed in r175528. Thanks for fixing this, the test case should
make sure that it won't break as badly again.

- Ben
> 
> 
> Thanks,
> yiannis
> 
> [1]: http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-February/059335.html
> 
> -- 
> Yiannis Tsiouris
> Ph.D. student,
> Software Engineering Laboratory,
> National Technical University of Athens
> WWW: http://www.softlab.ntua.gr/~gtsiour
> 
> <fix-gcmetadata.patch>_______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev