On Wed, Jan 31, 2024 at 10:11 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Wed, Jan 31, 2024 at 09:39:12AM -0800, H.J. Lu wrote:
> > GNU binutils has no issues with it:
>
> I know, I meant gcc.
> If I try the proposed:
> --- gcc/varasm.cc.jj    2024-01-30 08:44:43.304175273 +0100
> +++ gcc/varasm.cc       2024-01-31 18:45:57.271087170 +0100
> @@ -7459,15 +7459,46 @@ default_elf_select_rtx_section (machine_
>  {
>    int reloc = compute_reloc_for_rtx (x);
>
> +  tree decl = nullptr;
> +
> +  /* If it is a private COMDAT function symbol reference, call
> +     function_rodata_section for the read-only or relocated read-only
> +     data section associated with function DECL so that the COMDAT
> +     section will be used for the private COMDAT function symbol.  */
> +  if (HAVE_COMDAT_GROUP)
> +    {
> +      if (GET_CODE (x) == CONST
> +        && GET_CODE (XEXP (x, 0)) == PLUS
> +        && CONST_INT_P (XEXP (XEXP (x, 0), 1)))
> +       x = XEXP (XEXP (x, 0), 0);
> +
> +      if (GET_CODE (x) == SYMBOL_REF)
> +       {
> +        decl = SYMBOL_REF_DECL (x);
> +        if (decl
> +            && (TREE_CODE (decl) != FUNCTION_DECL
> +                || !DECL_COMDAT_GROUP (decl)
> +                || TREE_PUBLIC (decl)))
> +          decl = nullptr;
> +       }
> +    }
> +
>    /* ??? Handle small data here somehow.  */
>
>    if (reloc & targetm.asm_out.reloc_rw_mask ())
>      {
> +      if (decl)
> +       return get_section (reloc == 1
> +                           ? ".data.rel.ro.local" : ".data.rel.ro",
> +                           SECTION_WRITE | SECTION_RELRO | SECTION_LINKONCE,
> +                           decl);
>        if (reloc == 1)
>         return get_named_section (NULL, ".data.rel.ro.local", 1);
>        else
>         return get_named_section (NULL, ".data.rel.ro", 3);
>      }
> +  else if (decl)
> +    return get_section (".rodata", SECTION_LINKONCE, decl);
>
>    return mergeable_constant_section (mode, align, 0);
>  }
>
> and append
> typedef unsigned long int VV __attribute__((vector_size (2 * sizeof (long))));
> VV vv;
> __attribute__((noipa)) static void fn1 (void) {}
> __attribute__((noipa)) static void fn2 (void) {}
>
> void
> fn3 ()
> {
>   VV a = { (unsigned long) &fn1, (unsigned long) &fn2 };
>   vv = a;
> }
> to the first testcase (this is just to get a normal non-comdat
> .data.rel.ro.local section referencing non-comdat non-public syumbol),
> then I get the
> pr113617.C:19:1: error: section type conflict with ‘static bool R::B<_R(_A ...), _F>::F(R::H&, const R::H&, R::G) [with _R = void; _F = R::I<void (*(N1::N2::N3::C<{anonymous}::D<long long int>, false>*, long long int, long long int, long long int))(void*, long long int, long long int, long long int)>; _A = {}]’
>    19 | }
>       | ^
> In file included from pr113617.C:1:
> pr113617.h:21:15: note: ‘static bool R::B<_R(_A ...), _F>::F(R::H&, const R::H&, R::G) [with _R = void; _F = R::I<void (*(N1::N2::N3::C<{anonymous}::D<long long int>, false>*, long long int, long long int, long long int))(void*, long long int, long long int, long long int)>; _A = {}]’ was declared here
>    21 |   static bool F(H &, const H &, G) { return false; }
>       |               ^
> I feared.
> So, it seems get_section handles section purely by name lookup
> and isn't prepared to deal with multiple different sections
> of the same name, but different comdat group.
>
> Thus, maybe at least temporarily we need to use unique
> section names here, say
> .data.rel.ro.local.pool.<comdat_name>
> .data.rel.ro.pool.<comdat_name>
> .rodata.pool.<comdat_name>
> where <comdat_name> would be the name of the comdat group, i.e.
> IDENTIFIER_POINTER (DECL_COMDAT_GROUP (decl))
>
>         Jakub
>

I am testing this patch.

-- 
H.J.