Review of COMDAT patch

Mark Mitchell mark@codesourcery.com
Tue Aug 24 16:16:00 GMT 2004


HJ --

I've reviewed this patch:

  http://gcc.gnu.org/ml/gcc-patches/2004-08/txt00060.txt

in which you provide support for COMDAT.

First, a matter of style: the documentation for elf_comdat_group needs 
to say what each parameter is for and what value is returned.  Like 
"Given a section with the indicated NAME, sets *SECTION to ... and 
*GROUP to ...   Returns ...".

Second, I do not understand why default_elf_asm_named_section is testing 
HAVE_GAS_COMDAT_GROUP when deciding whether or not to put out the 
initial declaration of the section with 
named_section_first_declaration.  That code would seem to still apply to 
non-COMDAT sections, at least.

Why are you changing .gnu.linkonce.sb2 into ".sbss2", rahter than just 
".sb2"?  And the name of the section for the COMDAT group should be the 
name of the symbol, I think, not ".sbss2".  What's the logic there?

-- 
Mark Mitchell
CodeSourcery, LLC
(916) 791-8304
mark@codesourcery.com



More information about the Gcc-patches mailing list