Re: [PATCH] Handle function symbol reference in readonly data section
On Tue, Jan 30, 2024 at 4:58 AM H.J. Lu wrote:
>
> On Tue, Jan 30, 2024 at 4:51 AM Jakub Jelinek wrote:
> >
> > On Mon, Jan 29, 2024 at 06:05:25PM -0800, H.J. Lu wrote:
> > > LRA may call forcce_const_mem on this insn in the function
> > >
> > > (gdb) call debug_tree (func_decl)
> > > > > type > > type > > align:8 warn_if_not_align:0 symtab:0 alias-set -1
> > > canonical-type 0x77690f18
> > > pointer_to_this >
> > > QI
> > > size
> > > unit-size
> > > align:8 warn_if_not_align:0 symtab:0 alias-set -1
> > > canonical-type 0x727512a0 method basetype > > 0x7264b0a8 function_summary>
> > > arg-types > > 0x726887e0>
> > > chain > > 0x7264b2a0>
> > > chain > > purpose
> > > value
> > > chain > > 0x77690f18 void>
> > > pointer_to_this >
> > > addressable asm_written used nothrow public static weak decl_5 QI
> > > /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:268:1
> > > align:16 warn_if_not_align:0 context > > function_summary> initial abstract_origin
> > >
> > > result > > 0x77690f18 void>
> > > ignored VOID
> > > /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:269:20
> > > align:8 warn_if_not_align:0 context > > 0x72770900 __ct_base >>
> > > full-name "function_summary::function_summary(symbol_table*,
> > > bool = false) [with T = clone_info]"
> > > template-info > > template > > 0x72701540>
> > > VOID
> > > /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:268:1
> > > align:1 warn_if_not_align:0 context > > 0x727010a8 function_summary> result > > __ct >
> > > parms > > 0x7768a2d0 1>
> > > value > > 0x72646e00 function_summary>
> > > length:1
> > > elt:0 > > 0x726f5c78 T
> > > full-name "template
> > > function_summary::function_summary(symbol_table*, bool)">
> > > args > > 0x72987930 clone_info>>>
> > > use_template=1
> > > arguments > > type > > 0x7264b0a8 function_summary>
> > > readonly sizes-gimplified public unsigned DI
> > > size
> > > unit-size
> > > align:64 warn_if_not_align:0 symtab:0 alias-set -1
> > > canonical-type 0x72751348>
> > > readonly used unsigned read DI
> > > /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:269:20 size
> > > unit-size > > 8>
> > > align:64 warn_if_not_align:0 context > > 0x72770900 __ct_base > abstract_origin > > this>
> > > (reg/f:DI 117 [ this ]) arg-type
> > > incoming-rtl (reg:DI 5 di [ this ])
> > > chain > > 0x7264b2a0>
> > > used unsigned DI
> > > /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:268:56 size
> > > unit-size > > 8>
> > > align:64 warn_if_not_align:0 context > > 0x72770900 __ct_base > abstract_origin > > symtab>
> > > (reg/v/f:DI 118 [ symtab ]) arg-type > > 0x7264b2a0>
> > > incoming-rtl (reg:DI 4 si [ symtab ]) chain > > 0x72773880 ggc>>>
> > > struct-function 0x722cb228 chain > > __ct_comp >>
> > > (gdb)
> > >
> > > in gcc master branch tree:
> > >
> > > (gdb) call debug_rtx (curr_insn)
> > > (insn 14 128 15 2 (set (reg:V2DI 121 [ _21 ])
> > > (vec_concat:V2DI (symbol_ref/i:DI
> > > ("_ZN16function_summaryIP10clone_infoE16symtab_insertionEP11cgraph_nodePv")
> > > [flags 0x3] )
> > > (symbol_ref/i:DI
> > > ("_ZN16function_summaryIP10clone_infoE14symtab_removalEP11cgraph_nodePv")
> > > [flags 0x3] )))
> > > "/export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h":36:22
> > > 7521 {vec_concatv2di}
> > > (expr_list:REG_DEAD (reg/f:DI 123)
> > > (expr_list:REG_DEAD (reg/f:DI 122)
> > > (expr_list:REG_EQUIV (vec_concat:V2DI (symbol_ref/i:DI
> > > ("_ZN16function_summaryIP10clone_infoE16symtab_insertionEP11cgraph_nodePv")
> > > [flags 0x3] )
> > > (symbol_ref/i:DI
> > > ("_ZN16function_summaryIP10clone_infoE14symtab_removalEP11cgraph_nodePv")
> > > [flags 0x3] ))
> > > (nil)
> > > (gdb)
> > >
> > > The referenced symbol,
> > > function_summary::symtab_removal(cgraph_node*, void*),
> > > and the referencing function are in different COMDAT groups.
> >
> > And is the referenced symbol non-public? If so, how does that work?
> >
>
> I didn't check if it is public or private. It is OK for public, but not OK
> for private if they are in different comdat groups.
>
In GCC master source, when we force a comdat function symbol,
which may be in the same comdat group or a different comdat group,
into a constant pool, the symbol is always public. GCC never references
a private comdat function symbol in a different comdat group. We
Re: [PATCH] Handle function symbol reference in readonly data section
On Tue, Jan 30, 2024 at 4:51 AM Jakub Jelinek wrote:
>
> On Mon, Jan 29, 2024 at 06:05:25PM -0800, H.J. Lu wrote:
> > LRA may call forcce_const_mem on this insn in the function
> >
> > (gdb) call debug_tree (func_decl)
> > > type > type > align:8 warn_if_not_align:0 symtab:0 alias-set -1
> > canonical-type 0x77690f18
> > pointer_to_this >
> > QI
> > size
> > unit-size
> > align:8 warn_if_not_align:0 symtab:0 alias-set -1
> > canonical-type 0x727512a0 method basetype > 0x7264b0a8 function_summary>
> > arg-types > 0x726887e0>
> > chain > 0x7264b2a0>
> > chain > purpose
> > value
> > chain > 0x77690f18 void>
> > pointer_to_this >
> > addressable asm_written used nothrow public static weak decl_5 QI
> > /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:268:1
> > align:16 warn_if_not_align:0 context > function_summary> initial abstract_origin
> >
> > result > 0x77690f18 void>
> > ignored VOID
> > /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:269:20
> > align:8 warn_if_not_align:0 context > 0x72770900 __ct_base >>
> > full-name "function_summary::function_summary(symbol_table*,
> > bool = false) [with T = clone_info]"
> > template-info > template > 0x72701540>
> > VOID
> > /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:268:1
> > align:1 warn_if_not_align:0 context > 0x727010a8 function_summary> result > __ct >
> > parms > 0x7768a2d0 1>
> > value > 0x72646e00 function_summary>
> > length:1
> > elt:0 > 0x726f5c78 T
> > full-name "template
> > function_summary::function_summary(symbol_table*, bool)">
> > args > 0x72987930 clone_info>>>
> > use_template=1
> > arguments > type > 0x7264b0a8 function_summary>
> > readonly sizes-gimplified public unsigned DI
> > size
> > unit-size
> > align:64 warn_if_not_align:0 symtab:0 alias-set -1
> > canonical-type 0x72751348>
> > readonly used unsigned read DI
> > /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:269:20 size
> > unit-size > 8>
> > align:64 warn_if_not_align:0 context > 0x72770900 __ct_base > abstract_origin > this>
> > (reg/f:DI 117 [ this ]) arg-type
> > incoming-rtl (reg:DI 5 di [ this ])
> > chain > 0x7264b2a0>
> > used unsigned DI
> > /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:268:56 size
> > unit-size > 8>
> > align:64 warn_if_not_align:0 context > 0x72770900 __ct_base > abstract_origin > symtab>
> > (reg/v/f:DI 118 [ symtab ]) arg-type > 0x7264b2a0>
> > incoming-rtl (reg:DI 4 si [ symtab ]) chain > 0x72773880 ggc>>>
> > struct-function 0x722cb228 chain > __ct_comp >>
> > (gdb)
> >
> > in gcc master branch tree:
> >
> > (gdb) call debug_rtx (curr_insn)
> > (insn 14 128 15 2 (set (reg:V2DI 121 [ _21 ])
> > (vec_concat:V2DI (symbol_ref/i:DI
> > ("_ZN16function_summaryIP10clone_infoE16symtab_insertionEP11cgraph_nodePv")
> > [flags 0x3] )
> > (symbol_ref/i:DI
> > ("_ZN16function_summaryIP10clone_infoE14symtab_removalEP11cgraph_nodePv")
> > [flags 0x3] )))
> > "/export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h":36:22
> > 7521 {vec_concatv2di}
> > (expr_list:REG_DEAD (reg/f:DI 123)
> > (expr_list:REG_DEAD (reg/f:DI 122)
> > (expr_list:REG_EQUIV (vec_concat:V2DI (symbol_ref/i:DI
> > ("_ZN16function_summaryIP10clone_infoE16symtab_insertionEP11cgraph_nodePv")
> > [flags 0x3] )
> > (symbol_ref/i:DI
> > ("_ZN16function_summaryIP10clone_infoE14symtab_removalEP11cgraph_nodePv")
> > [flags 0x3] ))
> > (nil)
> > (gdb)
> >
> > The referenced symbol,
> > function_summary::symtab_removal(cgraph_node*, void*),
> > and the referencing function are in different COMDAT groups.
>
> And is the referenced symbol non-public? If so, how does that work?
>
I didn't check if it is public or private. It is OK for public, but not OK
for private if they are in different comdat groups.
--
H.J.
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 06:05:25PM -0800, H.J. Lu wrote:
> LRA may call forcce_const_mem on this insn in the function
>
> (gdb) call debug_tree (func_decl)
> type type align:8 warn_if_not_align:0 symtab:0 alias-set -1
> canonical-type 0x77690f18
> pointer_to_this >
> QI
> size
> unit-size
> align:8 warn_if_not_align:0 symtab:0 alias-set -1
> canonical-type 0x727512a0 method basetype 0x7264b0a8 function_summary>
> arg-types 0x726887e0>
> chain 0x7264b2a0>
> chain purpose
> value
> chain 0x77690f18 void>
> pointer_to_this >
> addressable asm_written used nothrow public static weak decl_5 QI
> /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:268:1
> align:16 warn_if_not_align:0 context function_summary> initial abstract_origin
>
> result 0x77690f18 void>
> ignored VOID
> /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:269:20
> align:8 warn_if_not_align:0 context 0x72770900 __ct_base >>
> full-name "function_summary::function_summary(symbol_table*,
> bool = false) [with T = clone_info]"
> template-info template 0x72701540>
> VOID
> /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:268:1
> align:1 warn_if_not_align:0 context 0x727010a8 function_summary> result __ct >
> parms 0x7768a2d0 1>
> value 0x72646e00 function_summary>
> length:1
> elt:0 0x726f5c78 T
> full-name "template
> function_summary::function_summary(symbol_table*, bool)">
> args 0x72987930 clone_info>>>
> use_template=1
> arguments type 0x7264b0a8 function_summary>
> readonly sizes-gimplified public unsigned DI
> size
> unit-size
> align:64 warn_if_not_align:0 symtab:0 alias-set -1
> canonical-type 0x72751348>
> readonly used unsigned read DI
> /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:269:20 size
> unit-size 8>
> align:64 warn_if_not_align:0 context 0x72770900 __ct_base > abstract_origin this>
> (reg/f:DI 117 [ this ]) arg-type
> incoming-rtl (reg:DI 5 di [ this ])
> chain 0x7264b2a0>
> used unsigned DI
> /export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:268:56 size
> unit-size 8>
> align:64 warn_if_not_align:0 context 0x72770900 __ct_base > abstract_origin symtab>
> (reg/v/f:DI 118 [ symtab ]) arg-type
> incoming-rtl (reg:DI 4 si [ symtab ]) chain 0x72773880 ggc>>>
> struct-function 0x722cb228 chain __ct_comp >>
> (gdb)
>
> in gcc master branch tree:
>
> (gdb) call debug_rtx (curr_insn)
> (insn 14 128 15 2 (set (reg:V2DI 121 [ _21 ])
> (vec_concat:V2DI (symbol_ref/i:DI
> ("_ZN16function_summaryIP10clone_infoE16symtab_insertionEP11cgraph_nodePv")
> [flags 0x3] )
> (symbol_ref/i:DI
> ("_ZN16function_summaryIP10clone_infoE14symtab_removalEP11cgraph_nodePv")
> [flags 0x3] )))
> "/export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h":36:22
> 7521 {vec_concatv2di}
> (expr_list:REG_DEAD (reg/f:DI 123)
> (expr_list:REG_DEAD (reg/f:DI 122)
> (expr_list:REG_EQUIV (vec_concat:V2DI (symbol_ref/i:DI
> ("_ZN16function_summaryIP10clone_infoE16symtab_insertionEP11cgraph_nodePv")
> [flags 0x3] )
> (symbol_ref/i:DI
> ("_ZN16function_summaryIP10clone_infoE14symtab_removalEP11cgraph_nodePv")
> [flags 0x3] ))
> (nil)
> (gdb)
>
> The referenced symbol,
> function_summary::symtab_removal(cgraph_node*, void*),
> and the referencing function are in different COMDAT groups.
And is the referenced symbol non-public? If so, how does that work?
Jakub
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 3:12 PM H.J. Lu wrote:
>
> On Mon, Jan 29, 2024 at 2:51 PM Jakub Jelinek wrote:
> >
> > On Mon, Jan 29, 2024 at 11:29:22PM +0100, Jakub Jelinek wrote:
> > > On Mon, Jan 29, 2024 at 11:22:44PM +0100, Jakub Jelinek wrote:
> > > > On Mon, Jan 29, 2024 at 02:01:56PM -0800, H.J. Lu wrote:
> > > > > > A function accesses a function symbol defined in a comdat group.
> > > > > > If the function symbol is public, any comdat definition of the same
> > > > > > group
> > > > > > signature should provide the function definition. If the function
> > > > > > symbol
> > > > > > is private to the comdat group, only functions in the same comdat
> > > > > > group can access the private function symbol. If a function in a
> > > > > > different
> > > > > > comdat group accesses a private symbol, it is a compiler bug and
> > > > > > link may catch it like in this case.
> > > > > >
> > > > >
> > > > > My patch simply puts the constant pool of the function symbol
> > > > > reference
> > > > > in the same comdat group as the function definition. I believe it is
> > > > > the
> > > > > right thing to do.
> > > >
> > > > I disagree, I think we should use something like
> > > > if (current_function_decl)
> > >
> > > Or perhaps && DECL_COMDAT_GROUP (current_function_decl) added here as
> > > well,
> > > just to make it change things less often.
> > >
> > > > return targetm.asm_out.function_rodata_section
> > > > (current_function_decl,
> > > > true);
> > > >
> > > > Obviously, for non-reloc or non-pic, we don't want an unconditional
> > > > if (current_function_decl)
> > > > return targetm.asm_out.function_rodata_section
> > > > (current_function_decl,
> > > > false);
> > > > that would kill mergeable sections, so perhaps
> > > > if (current_function_decl
> > > > && reloc
> > > > && DECL_COMDAT_GROUP (current_function_decl))
> > > > return targetm.asm_out.function_rodata_section
> > > > (current_function_decl,
> > > > false);
> >
> > Now, that doesn't actually work, because current_function_decl is always
> > NULL when the constant pool entries are emitted.
> > But basing the output section on what it refers rather than what refers to
> > it seems wrong, plus there is the section anchors support, which treats them
> > yet differently.
> > So, I wonder if force_const_mem shouldn't punt if asked to emit from
LRA may call forcce_const_mem on this insn in the function
(gdb) call debug_tree (func_decl)
>
QI
size
unit-size
align:8 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x727512a0 method basetype
arg-types
chain
chain
value
chain
pointer_to_this >
addressable asm_written used nothrow public static weak decl_5 QI
/export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:268:1
align:16 warn_if_not_align:0 context initial abstract_origin
result
ignored VOID
/export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:269:20
align:8 warn_if_not_align:0 context >
full-name "function_summary::function_summary(symbol_table*,
bool = false) [with T = clone_info]"
template-info
VOID
/export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:268:1
align:1 warn_if_not_align:0 context result
parms
value
length:1
elt:0 >>>
full-name "template
function_summary::function_summary(symbol_table*, bool)">
args >>
use_template=1
arguments
readonly sizes-gimplified public unsigned DI
size
unit-size
align:64 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x72751348>
readonly used unsigned read DI
/export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:269:20 size
unit-size
align:64 warn_if_not_align:0 context abstract_origin
(reg/f:DI 117 [ this ]) arg-type
incoming-rtl (reg:DI 5 di [ this ])
chain
used unsigned DI
/export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h:268:56 size
unit-size
align:64 warn_if_not_align:0 context abstract_origin
(reg/v/f:DI 118 [ symtab ]) arg-type
incoming-rtl (reg:DI 4 si [ symtab ]) chain >>
struct-function 0x722cb228 chain >
(gdb)
in gcc master branch tree:
(gdb) call debug_rtx (curr_insn)
(insn 14 128 15 2 (set (reg:V2DI 121 [ _21 ])
(vec_concat:V2DI (symbol_ref/i:DI
("_ZN16function_summaryIP10clone_infoE16symtab_insertionEP11cgraph_nodePv")
[flags 0x3] )
(symbol_ref/i:DI
("_ZN16function_summaryIP10clone_infoE14symtab_removalEP11cgraph_nodePv")
[flags 0x3] )))
"/export/gnu/import/git/gitlab/x86-gcc/gcc/symbol-summary.h":36:22
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 2:51 PM Jakub Jelinek wrote: > > On Mon, Jan 29, 2024 at 11:29:22PM +0100, Jakub Jelinek wrote: > > On Mon, Jan 29, 2024 at 11:22:44PM +0100, Jakub Jelinek wrote: > > > On Mon, Jan 29, 2024 at 02:01:56PM -0800, H.J. Lu wrote: > > > > > A function accesses a function symbol defined in a comdat group. > > > > > If the function symbol is public, any comdat definition of the same > > > > > group > > > > > signature should provide the function definition. If the function > > > > > symbol > > > > > is private to the comdat group, only functions in the same comdat > > > > > group can access the private function symbol. If a function in a > > > > > different > > > > > comdat group accesses a private symbol, it is a compiler bug and > > > > > link may catch it like in this case. > > > > > > > > > > > > > My patch simply puts the constant pool of the function symbol reference > > > > in the same comdat group as the function definition. I believe it is > > > > the > > > > right thing to do. > > > > > > I disagree, I think we should use something like > > > if (current_function_decl) > > > > Or perhaps && DECL_COMDAT_GROUP (current_function_decl) added here as well, > > just to make it change things less often. > > > > > return targetm.asm_out.function_rodata_section (current_function_decl, > > > true); > > > > > > Obviously, for non-reloc or non-pic, we don't want an unconditional > > > if (current_function_decl) > > > return targetm.asm_out.function_rodata_section (current_function_decl, > > > false); > > > that would kill mergeable sections, so perhaps > > > if (current_function_decl > > > && reloc > > > && DECL_COMDAT_GROUP (current_function_decl)) > > > return targetm.asm_out.function_rodata_section (current_function_decl, > > > false); > > Now, that doesn't actually work, because current_function_decl is always > NULL when the constant pool entries are emitted. > But basing the output section on what it refers rather than what refers to > it seems wrong, plus there is the section anchors support, which treats them > yet differently. > So, I wonder if force_const_mem shouldn't punt if asked to emit from > DECL_COMDAT_GROUP (current_function_decl) a SYMBOL_REF (or CONST PLUS > SYMBOL_REF ...) with the same DECL_COMDAT_GROUP with a private symbol, > or shouldn't punt unless using a per-function (i.e. non-shared) constant > pool, or force a per-function constant pool in that case somehow. > Here is the patch to only call function_rodata_section for COMDAT function symbol reference: https://patchwork.sourceware.org/project/gcc/list/?series=30329 -- H.J.
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 11:29:22PM +0100, Jakub Jelinek wrote: > On Mon, Jan 29, 2024 at 11:22:44PM +0100, Jakub Jelinek wrote: > > On Mon, Jan 29, 2024 at 02:01:56PM -0800, H.J. Lu wrote: > > > > A function accesses a function symbol defined in a comdat group. > > > > If the function symbol is public, any comdat definition of the same > > > > group > > > > signature should provide the function definition. If the function > > > > symbol > > > > is private to the comdat group, only functions in the same comdat > > > > group can access the private function symbol. If a function in a > > > > different > > > > comdat group accesses a private symbol, it is a compiler bug and > > > > link may catch it like in this case. > > > > > > > > > > My patch simply puts the constant pool of the function symbol reference > > > in the same comdat group as the function definition. I believe it is the > > > right thing to do. > > > > I disagree, I think we should use something like > > if (current_function_decl) > > Or perhaps && DECL_COMDAT_GROUP (current_function_decl) added here as well, > just to make it change things less often. > > > return targetm.asm_out.function_rodata_section (current_function_decl, > > true); > > > > Obviously, for non-reloc or non-pic, we don't want an unconditional > > if (current_function_decl) > > return targetm.asm_out.function_rodata_section (current_function_decl, > > false); > > that would kill mergeable sections, so perhaps > > if (current_function_decl > > && reloc > > && DECL_COMDAT_GROUP (current_function_decl)) > > return targetm.asm_out.function_rodata_section (current_function_decl, > > false); Now, that doesn't actually work, because current_function_decl is always NULL when the constant pool entries are emitted. But basing the output section on what it refers rather than what refers to it seems wrong, plus there is the section anchors support, which treats them yet differently. So, I wonder if force_const_mem shouldn't punt if asked to emit from DECL_COMDAT_GROUP (current_function_decl) a SYMBOL_REF (or CONST PLUS SYMBOL_REF ...) with the same DECL_COMDAT_GROUP with a private symbol, or shouldn't punt unless using a per-function (i.e. non-shared) constant pool, or force a per-function constant pool in that case somehow. Jakub
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 11:22:44PM +0100, Jakub Jelinek wrote: > On Mon, Jan 29, 2024 at 02:01:56PM -0800, H.J. Lu wrote: > > > A function accesses a function symbol defined in a comdat group. > > > If the function symbol is public, any comdat definition of the same group > > > signature should provide the function definition. If the function symbol > > > is private to the comdat group, only functions in the same comdat > > > group can access the private function symbol. If a function in a > > > different > > > comdat group accesses a private symbol, it is a compiler bug and > > > link may catch it like in this case. > > > > > > > My patch simply puts the constant pool of the function symbol reference > > in the same comdat group as the function definition. I believe it is the > > right thing to do. > > I disagree, I think we should use something like > if (current_function_decl) Or perhaps && DECL_COMDAT_GROUP (current_function_decl) added here as well, just to make it change things less often. > return targetm.asm_out.function_rodata_section (current_function_decl, > true); > > Obviously, for non-reloc or non-pic, we don't want an unconditional > if (current_function_decl) > return targetm.asm_out.function_rodata_section (current_function_decl, > false); > that would kill mergeable sections, so perhaps > if (current_function_decl > && reloc > && DECL_COMDAT_GROUP (current_function_decl)) > return targetm.asm_out.function_rodata_section (current_function_decl, > false); Jakub
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 02:01:56PM -0800, H.J. Lu wrote: > > A function accesses a function symbol defined in a comdat group. > > If the function symbol is public, any comdat definition of the same group > > signature should provide the function definition. If the function symbol > > is private to the comdat group, only functions in the same comdat > > group can access the private function symbol. If a function in a different > > comdat group accesses a private symbol, it is a compiler bug and > > link may catch it like in this case. > > > > My patch simply puts the constant pool of the function symbol reference > in the same comdat group as the function definition. I believe it is the > right thing to do. I disagree, I think we should use something like if (current_function_decl) return targetm.asm_out.function_rodata_section (current_function_decl, true); Obviously, for non-reloc or non-pic, we don't want an unconditional if (current_function_decl) return targetm.asm_out.function_rodata_section (current_function_decl, false); that would kill mergeable sections, so perhaps if (current_function_decl && reloc && DECL_COMDAT_GROUP (current_function_decl)) return targetm.asm_out.function_rodata_section (current_function_decl, false); Jakub
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 2:01 PM H.J. Lu wrote:
>
> On Mon, Jan 29, 2024 at 1:42 PM H.J. Lu wrote:
> >
> > On Mon, Jan 29, 2024 at 1:22 PM H.J. Lu wrote:
> > >
> > > On Mon, Jan 29, 2024 at 1:00 PM H.J. Lu wrote:
> > > >
> > > > On Mon, Jan 29, 2024 at 9:34 AM H.J. Lu wrote:
> > > > >
> > > > > On Mon, Jan 29, 2024 at 9:00 AM Jakub Jelinek
> > > > > wrote:
> > > > > >
> > > > > > On Mon, Jan 29, 2024 at 08:45:45AM -0800, H.J. Lu wrote:
> > > > > > > In this case, these are internal to the same comdat group:
> > > > > >
> > > > > > But that is only by accident, no?
> > > > >
> > > > > This may be by luck. I don't know if gcc checks it when
> > > > > generating such references.
> > > > >
> > > > > > I mean, if you need to refer to such a symbol from
> > > > > > non-comdat function or comdat function in a different comdat group
> > > > > > and RA decides it wants the constant in memory rather than code?
> > > > > > Your patch uses
> > > > > > if (decl)
> > > > > > return targetm.asm_out.function_rodata_section (decl, ???);
> > > > > > and default_function_rodata_section only looks at comdat group of
> > > > > > the
> > > > > > passed in decl. But the decl here is what the constant refers to,
> > > > > > not
> > > > > > who is referring it.
> > > >
> > > > LRA puts a function symbol reference in a constant pool via
> > > >
> > > > #0 force_const_mem (in_mode=E_DImode, x=0x7fffe9e7e000)
> > > > at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/varasm.cc:3951
> > > > #1 0x01833870 in curr_insn_transform (check_only_p=false)
> > > > at
> > > > /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra-constraints.cc:4473
> > > > #2 0x01836eae in lra_constraints (first_p=true)
> > > > at
> > > > /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra-constraints.cc:5462
> > > > #3 0x0181fcf1 in lra (f=0x0, verbose=5)
> > > > at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra.cc:2442
> > > > #4 0x017c8828 in do_reload ()
> > > > at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/ira.cc:5973
> > > > #5 0x017c8d25 in (anonymous namespace)::pass_reload::execute (
> > > > this=0x48d8730)
> > > > at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/ira.cc:6161
> > > >
> > > > for
> > > >
> > > > (gdb) call debug_rtx (curr_insn)
> > > > (insn 12 57 15 2 (set (reg:V2DI 101 [ _16 ])
> > > > (vec_concat:V2DI (symbol_ref:DI
> > > > ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE")
> > > > [flags 0x3] )
> > > > (reg/f:DI 109))) 7521 {vec_concatv2di}
> > > > (expr_list:REG_DEAD (reg/f:DI 110)
> > > > (expr_list:REG_DEAD (reg/f:DI 109)
> > > > (expr_list:REG_EQUIV (vec_concat:V2DI (symbol_ref:DI
> > > > ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE")
> > > > [flags 0x3] )
> > > > (symbol_ref:DI
> > > > ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx9_M_invokeERKNS_9_Any_dataE")
> > > > [flags 0x3] ))
> > > > (nil)
> > > > (gdb)
> > > >
> > > > CONST_POOL_OK_P doesn't check if it is safe to do so for function
> > > > symbols. Here is a patch to add the check.
> > > >
> > > > --
> > > > H.J.
> > >
> > > On the other hand, does C++ even allow access to non-public members
> > > from different classes? So my patch should be safe and linker should
> > > catch all invalid comdat usages like this bug.
> >
> > A function accesses a function symbol defined in a comdat group.
> > If the function symbol is public, any comdat definition of the same group
> > signature should provide the function definition. If the function symbol
> > is private to the comdat group, only functions in the same comdat
> > group can access the private function symbol. If a function in a different
> > comdat group accesses a private symbol, it is a compiler bug and
> > link may catch it like in this case.
> >
>
> My patch simply puts the constant pool of the function symbol reference
> in the same comdat group as the function definition. I believe it is the
> right thing to do.
If we are concerned that not all comdat definitions provide such a constant
pool, we can change LA to only allow such a constant pool when it is safe
to do so.
--
H.J.
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 1:42 PM H.J. Lu wrote:
>
> On Mon, Jan 29, 2024 at 1:22 PM H.J. Lu wrote:
> >
> > On Mon, Jan 29, 2024 at 1:00 PM H.J. Lu wrote:
> > >
> > > On Mon, Jan 29, 2024 at 9:34 AM H.J. Lu wrote:
> > > >
> > > > On Mon, Jan 29, 2024 at 9:00 AM Jakub Jelinek wrote:
> > > > >
> > > > > On Mon, Jan 29, 2024 at 08:45:45AM -0800, H.J. Lu wrote:
> > > > > > In this case, these are internal to the same comdat group:
> > > > >
> > > > > But that is only by accident, no?
> > > >
> > > > This may be by luck. I don't know if gcc checks it when
> > > > generating such references.
> > > >
> > > > > I mean, if you need to refer to such a symbol from
> > > > > non-comdat function or comdat function in a different comdat group
> > > > > and RA decides it wants the constant in memory rather than code?
> > > > > Your patch uses
> > > > > if (decl)
> > > > > return targetm.asm_out.function_rodata_section (decl, ???);
> > > > > and default_function_rodata_section only looks at comdat group of the
> > > > > passed in decl. But the decl here is what the constant refers to, not
> > > > > who is referring it.
> > >
> > > LRA puts a function symbol reference in a constant pool via
> > >
> > > #0 force_const_mem (in_mode=E_DImode, x=0x7fffe9e7e000)
> > > at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/varasm.cc:3951
> > > #1 0x01833870 in curr_insn_transform (check_only_p=false)
> > > at
> > > /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra-constraints.cc:4473
> > > #2 0x01836eae in lra_constraints (first_p=true)
> > > at
> > > /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra-constraints.cc:5462
> > > #3 0x0181fcf1 in lra (f=0x0, verbose=5)
> > > at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra.cc:2442
> > > #4 0x017c8828 in do_reload ()
> > > at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/ira.cc:5973
> > > #5 0x017c8d25 in (anonymous namespace)::pass_reload::execute (
> > > this=0x48d8730)
> > > at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/ira.cc:6161
> > >
> > > for
> > >
> > > (gdb) call debug_rtx (curr_insn)
> > > (insn 12 57 15 2 (set (reg:V2DI 101 [ _16 ])
> > > (vec_concat:V2DI (symbol_ref:DI
> > > ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE")
> > > [flags 0x3] )
> > > (reg/f:DI 109))) 7521 {vec_concatv2di}
> > > (expr_list:REG_DEAD (reg/f:DI 110)
> > > (expr_list:REG_DEAD (reg/f:DI 109)
> > > (expr_list:REG_EQUIV (vec_concat:V2DI (symbol_ref:DI
> > > ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE")
> > > [flags 0x3] )
> > > (symbol_ref:DI
> > > ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx9_M_invokeERKNS_9_Any_dataE")
> > > [flags 0x3] ))
> > > (nil)
> > > (gdb)
> > >
> > > CONST_POOL_OK_P doesn't check if it is safe to do so for function
> > > symbols. Here is a patch to add the check.
> > >
> > > --
> > > H.J.
> >
> > On the other hand, does C++ even allow access to non-public members
> > from different classes? So my patch should be safe and linker should
> > catch all invalid comdat usages like this bug.
>
> A function accesses a function symbol defined in a comdat group.
> If the function symbol is public, any comdat definition of the same group
> signature should provide the function definition. If the function symbol
> is private to the comdat group, only functions in the same comdat
> group can access the private function symbol. If a function in a different
> comdat group accesses a private symbol, it is a compiler bug and
> link may catch it like in this case.
>
My patch simply puts the constant pool of the function symbol reference
in the same comdat group as the function definition. I believe it is the
right thing to do.
--
H.J.
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 1:22 PM H.J. Lu wrote:
>
> On Mon, Jan 29, 2024 at 1:00 PM H.J. Lu wrote:
> >
> > On Mon, Jan 29, 2024 at 9:34 AM H.J. Lu wrote:
> > >
> > > On Mon, Jan 29, 2024 at 9:00 AM Jakub Jelinek wrote:
> > > >
> > > > On Mon, Jan 29, 2024 at 08:45:45AM -0800, H.J. Lu wrote:
> > > > > In this case, these are internal to the same comdat group:
> > > >
> > > > But that is only by accident, no?
> > >
> > > This may be by luck. I don't know if gcc checks it when
> > > generating such references.
> > >
> > > > I mean, if you need to refer to such a symbol from
> > > > non-comdat function or comdat function in a different comdat group
> > > > and RA decides it wants the constant in memory rather than code?
> > > > Your patch uses
> > > > if (decl)
> > > > return targetm.asm_out.function_rodata_section (decl, ???);
> > > > and default_function_rodata_section only looks at comdat group of the
> > > > passed in decl. But the decl here is what the constant refers to, not
> > > > who is referring it.
> >
> > LRA puts a function symbol reference in a constant pool via
> >
> > #0 force_const_mem (in_mode=E_DImode, x=0x7fffe9e7e000)
> > at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/varasm.cc:3951
> > #1 0x01833870 in curr_insn_transform (check_only_p=false)
> > at
> > /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra-constraints.cc:4473
> > #2 0x01836eae in lra_constraints (first_p=true)
> > at
> > /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra-constraints.cc:5462
> > #3 0x0181fcf1 in lra (f=0x0, verbose=5)
> > at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra.cc:2442
> > #4 0x017c8828 in do_reload ()
> > at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/ira.cc:5973
> > #5 0x017c8d25 in (anonymous namespace)::pass_reload::execute (
> > this=0x48d8730)
> > at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/ira.cc:6161
> >
> > for
> >
> > (gdb) call debug_rtx (curr_insn)
> > (insn 12 57 15 2 (set (reg:V2DI 101 [ _16 ])
> > (vec_concat:V2DI (symbol_ref:DI
> > ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE")
> > [flags 0x3] )
> > (reg/f:DI 109))) 7521 {vec_concatv2di}
> > (expr_list:REG_DEAD (reg/f:DI 110)
> > (expr_list:REG_DEAD (reg/f:DI 109)
> > (expr_list:REG_EQUIV (vec_concat:V2DI (symbol_ref:DI
> > ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE")
> > [flags 0x3] )
> > (symbol_ref:DI
> > ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx9_M_invokeERKNS_9_Any_dataE")
> > [flags 0x3] ))
> > (nil)
> > (gdb)
> >
> > CONST_POOL_OK_P doesn't check if it is safe to do so for function
> > symbols. Here is a patch to add the check.
> >
> > --
> > H.J.
>
> On the other hand, does C++ even allow access to non-public members
> from different classes? So my patch should be safe and linker should
> catch all invalid comdat usages like this bug.
A function accesses a function symbol defined in a comdat group.
If the function symbol is public, any comdat definition of the same group
signature should provide the function definition. If the function symbol
is private to the comdat group, only functions in the same comdat
group can access the private function symbol. If a function in a different
comdat group accesses a private symbol, it is a compiler bug and
link may catch it like in this case.
--
H.J.
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 1:00 PM H.J. Lu wrote:
>
> On Mon, Jan 29, 2024 at 9:34 AM H.J. Lu wrote:
> >
> > On Mon, Jan 29, 2024 at 9:00 AM Jakub Jelinek wrote:
> > >
> > > On Mon, Jan 29, 2024 at 08:45:45AM -0800, H.J. Lu wrote:
> > > > In this case, these are internal to the same comdat group:
> > >
> > > But that is only by accident, no?
> >
> > This may be by luck. I don't know if gcc checks it when
> > generating such references.
> >
> > > I mean, if you need to refer to such a symbol from
> > > non-comdat function or comdat function in a different comdat group
> > > and RA decides it wants the constant in memory rather than code?
> > > Your patch uses
> > > if (decl)
> > > return targetm.asm_out.function_rodata_section (decl, ???);
> > > and default_function_rodata_section only looks at comdat group of the
> > > passed in decl. But the decl here is what the constant refers to, not
> > > who is referring it.
>
> LRA puts a function symbol reference in a constant pool via
>
> #0 force_const_mem (in_mode=E_DImode, x=0x7fffe9e7e000)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/varasm.cc:3951
> #1 0x01833870 in curr_insn_transform (check_only_p=false)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra-constraints.cc:4473
> #2 0x01836eae in lra_constraints (first_p=true)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra-constraints.cc:5462
> #3 0x0181fcf1 in lra (f=0x0, verbose=5)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra.cc:2442
> #4 0x017c8828 in do_reload ()
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/ira.cc:5973
> #5 0x017c8d25 in (anonymous namespace)::pass_reload::execute (
> this=0x48d8730)
> at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/ira.cc:6161
>
> for
>
> (gdb) call debug_rtx (curr_insn)
> (insn 12 57 15 2 (set (reg:V2DI 101 [ _16 ])
> (vec_concat:V2DI (symbol_ref:DI
> ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE")
> [flags 0x3] )
> (reg/f:DI 109))) 7521 {vec_concatv2di}
> (expr_list:REG_DEAD (reg/f:DI 110)
> (expr_list:REG_DEAD (reg/f:DI 109)
> (expr_list:REG_EQUIV (vec_concat:V2DI (symbol_ref:DI
> ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE")
> [flags 0x3] )
> (symbol_ref:DI
> ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx9_M_invokeERKNS_9_Any_dataE")
> [flags 0x3] ))
> (nil)
> (gdb)
>
> CONST_POOL_OK_P doesn't check if it is safe to do so for function
> symbols. Here is a patch to add the check.
>
> --
> H.J.
On the other hand, does C++ even allow access to non-public members
from different classes? So my patch should be safe and linker should
catch all invalid comdat usages like this bug.
--
H.J.
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 9:34 AM H.J. Lu wrote:
>
> On Mon, Jan 29, 2024 at 9:00 AM Jakub Jelinek wrote:
> >
> > On Mon, Jan 29, 2024 at 08:45:45AM -0800, H.J. Lu wrote:
> > > In this case, these are internal to the same comdat group:
> >
> > But that is only by accident, no?
>
> This may be by luck. I don't know if gcc checks it when
> generating such references.
>
> > I mean, if you need to refer to such a symbol from
> > non-comdat function or comdat function in a different comdat group
> > and RA decides it wants the constant in memory rather than code?
> > Your patch uses
> > if (decl)
> > return targetm.asm_out.function_rodata_section (decl, ???);
> > and default_function_rodata_section only looks at comdat group of the
> > passed in decl. But the decl here is what the constant refers to, not
> > who is referring it.
LRA puts a function symbol reference in a constant pool via
#0 force_const_mem (in_mode=E_DImode, x=0x7fffe9e7e000)
at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/varasm.cc:3951
#1 0x01833870 in curr_insn_transform (check_only_p=false)
at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra-constraints.cc:4473
#2 0x01836eae in lra_constraints (first_p=true)
at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra-constraints.cc:5462
#3 0x0181fcf1 in lra (f=0x0, verbose=5)
at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/lra.cc:2442
#4 0x017c8828 in do_reload ()
at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/ira.cc:5973
#5 0x017c8d25 in (anonymous namespace)::pass_reload::execute (
this=0x48d8730)
at /export/gnu/import/git/gitlab/x86-gcc-test/gcc/ira.cc:6161
for
(gdb) call debug_rtx (curr_insn)
(insn 12 57 15 2 (set (reg:V2DI 101 [ _16 ])
(vec_concat:V2DI (symbol_ref:DI
("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE")
[flags 0x3] )
(reg/f:DI 109))) 7521 {vec_concatv2di}
(expr_list:REG_DEAD (reg/f:DI 110)
(expr_list:REG_DEAD (reg/f:DI 109)
(expr_list:REG_EQUIV (vec_concat:V2DI (symbol_ref:DI
("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE")
[flags 0x3] )
(symbol_ref:DI
("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx9_M_invokeERKNS_9_Any_dataE")
[flags 0x3] ))
(nil)
(gdb)
CONST_POOL_OK_P doesn't check if it is safe to do so for function
symbols. Here is a patch to add the check.
--
H.J.
From 1947920740e48cdc8076299f8cc58e797ec39a7c Mon Sep 17 00:00:00 2001
From: "H.J. Lu"
Date: Mon, 29 Jan 2024 12:53:32 -0800
Subject: [PATCH] lra: Add const_pool_reference_ok
LRA may put a function symbol reference in
(gdb) call debug_rtx (curr_insn)
(insn 12 57 15 2 (set (reg:V2DI 101 [ _16 ])
(vec_concat:V2DI (symbol_ref:DI ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE") [flags 0x3] )
(reg/f:DI 109))) 7521 {vec_concatv2di}
(expr_list:REG_DEAD (reg/f:DI 110)
(expr_list:REG_DEAD (reg/f:DI 109)
(expr_list:REG_EQUIV (vec_concat:V2DI (symbol_ref:DI ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE") [flags 0x3] )
(symbol_ref:DI ("_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx9_M_invokeERKNS_9_Any_dataE") [flags 0x3] ))
(nil)
(gdb)
in the constant pool. But it isn't safe when the referenced function
symbol is in a different COMDAT group from the current instruction
function body if the function symbol isn't public.
Add const_pool_reference_ok to check if a function symbol can be forced
into the constant pool.
PR rtl-optimization/113617
* lra-constraints.cc (const_pool_reference_ok): New.
(CONST_POOL_OK_P): Use.
---
gcc/lra-constraints.cc | 30 ++
1 file changed, 30 insertions(+)
diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index 0ae81c1ff9c..59e6944c245 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -925,6 +925,35 @@ operands_match_p (rtx x, rtx y, int y_hard_regno)
return true;
}
+/* Return true if the symbol X can be referenced in the function
+ FUNC_DECL. */
+
+static bool
+const_pool_reference_ok (tree func_decl, rtx x)
+{
+ /* It is OK if there is no COMDAT or X
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 9:00 AM Jakub Jelinek wrote: > > On Mon, Jan 29, 2024 at 08:45:45AM -0800, H.J. Lu wrote: > > In this case, these are internal to the same comdat group: > > But that is only by accident, no? This may be by luck. I don't know if gcc checks it when generating such references. > I mean, if you need to refer to such a symbol from > non-comdat function or comdat function in a different comdat group > and RA decides it wants the constant in memory rather than code? > Your patch uses > if (decl) > return targetm.asm_out.function_rodata_section (decl, ???); > and default_function_rodata_section only looks at comdat group of the > passed in decl. But the decl here is what the constant refers to, not > who is referring it. > > Jakub > -- H.J.
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 08:45:45AM -0800, H.J. Lu wrote: > In this case, these are internal to the same comdat group: But that is only by accident, no? I mean, if you need to refer to such a symbol from non-comdat function or comdat function in a different comdat group and RA decides it wants the constant in memory rather than code? Your patch uses if (decl) return targetm.asm_out.function_rodata_section (decl, ???); and default_function_rodata_section only looks at comdat group of the passed in decl. But the decl here is what the constant refers to, not who is referring it. Jakub
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 8:34 AM Jakub Jelinek wrote: > > On Mon, Jan 29, 2024 at 08:23:21AM -0800, H.J. Lu wrote: > > > baz: > > > movq.LC0(%rip), %xmm0 > > > ret > > > > I don't think this is valid. We can't reference a non-public > > symbol outside of a COMDAT group. It is OK to reference > > foo or foo + 1, but not .LC0. > > But that is exactly what your patch does, e.g. on the first testcase: > --- pr113617-1a.s 2024-01-29 11:29:55.831512974 +0100 > +++ pr113617-1a.s 2024-01-29 11:30:04.335394116 +0100 > @@ -51,28 +-51,28 @@ > .section > .text._ZN3vtk6detail3smp15vtkSMPToolsImplILi1EE3ForINS1_27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EvxxxRT_,"axG",@progbits,_ZN26vtkStaticCellLinksTemplateIxE18ThreadedBuildLinksExxP12vtkCellArray,comdat > .align 2 > .p2align 4 > .type > _ZN3vtk6detail3smp15vtkSMPToolsImplILi1EE3ForINS1_27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EvxxxRT_, > @function > > _ZN3vtk6detail3smp15vtkSMPToolsImplILi1EE3ForINS1_27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EvxxxRT_: > pushq %r15 > leaq > _ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx9_M_invokeERKNS_9_Any_dataE(%rip), > %rax > leaq > _ZN3vtk6detail3smp23ExecuteFunctorSTDThreadINS1_27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EvPvxxx(%rip), > %r15 > pushq %r14 > movq%rax, %xmm1 > pushq %r13 > pushq %r12 > movq%rdx, %r12 > pushq %rbp > movq%r8, %rbp > pushq %rbx > movq%rcx, %rbx > subq$40, %rsp > movlFor_threadNumber(%rip), %esi > movq.LC0(%rip), %xmm0 > leaq31(%rsp), %r13 > punpcklqdq %xmm1, %xmm0 > movq%r13, %rdi > movaps %xmm0, (%rsp) > call_ZN3vtk6detail3smp16vtkSMPThreadPoolC1Ei@PLT > movq(%rsp), %r14 > .p2align 4,,10 > .p2align 3 > @@ -191,9 +191,9 @@ vtkConstrainedSmoothingFilterRequestData > .size For_threadNumber, 4 > For_threadNumber: > .zero 4 > - .section.data.rel.ro.local,"aw" > + .section > .data.rel.ro.local._ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE,"awG",@progbits,_ZN26vtkStaticCellLinksTemplateIxE18ThreadedBuildLinksExxP12vtkCellArray,comdat > .align 8 > .LC0: > .quad > _ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE > - .ident "GCC: (GNU) 14.0.1 20240127 (experimental)" > + .ident "GCC: (GNU) 14.0.1 20240129 (experimental)" > .section.note.GNU-stack,"",@progbits > > Jakub > In this case, these are internal to the same comdat group: .section .text._ZN3vtk6detail3smp15vtkSMPToolsImplILi1EE3ForINS1_27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EvxxxRT_,"axG",@progbits,_ZN26vtkStaticCellLinksTemplateIxE18ThreadedBuildLinksExxP12vtkCellArray,comdat .align 2 .p2align 4 .type _ZN3vtk6detail3smp15vtkSMPToolsImplILi1EE3ForINS1_27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EvxxxRT_, @function _ZN3vtk6detail3smp15vtkSMPToolsImplILi1EE3ForINS1_27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EvxxxRT_: .LFB27: .cfi_startproc ... movq .LC0(%rip), %xmm0 ... .section .text._ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE,"axG",@progbits,_ZN26vtkStaticCellLinksTemplateIxE18ThreadedBuildLinksExxP12vtkCellArray,comdat .p2align 4 .type _ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE, @function _ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE: .LFB34: .cfi_startproc xorl %eax, %eax ret .cfi_endproc ... .section .data.rel.ro.local._ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE,"awG",@progbits,_ZN26vtkStaticCellLinksTemplateIxE18ThreadedBuildLinksExxP12vtkCellArray,comdat .align 8 .LC0: .quad
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 08:23:21AM -0800, H.J. Lu wrote: > > baz: > > movq.LC0(%rip), %xmm0 > > ret > > I don't think this is valid. We can't reference a non-public > symbol outside of a COMDAT group. It is OK to reference > foo or foo + 1, but not .LC0. But that is exactly what your patch does, e.g. on the first testcase: --- pr113617-1a.s 2024-01-29 11:29:55.831512974 +0100 +++ pr113617-1a.s 2024-01-29 11:30:04.335394116 +0100 @@ -51,28 +-51,28 @@ .section .text._ZN3vtk6detail3smp15vtkSMPToolsImplILi1EE3ForINS1_27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EvxxxRT_,"axG",@progbits,_ZN26vtkStaticCellLinksTemplateIxE18ThreadedBuildLinksExxP12vtkCellArray,comdat .align 2 .p2align 4 .type _ZN3vtk6detail3smp15vtkSMPToolsImplILi1EE3ForINS1_27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EvxxxRT_, @function _ZN3vtk6detail3smp15vtkSMPToolsImplILi1EE3ForINS1_27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EvxxxRT_: pushq %r15 leaq _ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx9_M_invokeERKNS_9_Any_dataE(%rip), %rax leaq _ZN3vtk6detail3smp23ExecuteFunctorSTDThreadINS1_27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EvPvxxx(%rip), %r15 pushq %r14 movq%rax, %xmm1 pushq %r13 pushq %r12 movq%rdx, %r12 pushq %rbp movq%r8, %rbp pushq %rbx movq%rcx, %rbx subq$40, %rsp movlFor_threadNumber(%rip), %esi movq.LC0(%rip), %xmm0 leaq31(%rsp), %r13 punpcklqdq %xmm1, %xmm0 movq%r13, %rdi movaps %xmm0, (%rsp) call_ZN3vtk6detail3smp16vtkSMPThreadPoolC1Ei@PLT movq(%rsp), %r14 .p2align 4,,10 .p2align 3 @@ -191,9 +191,9 @@ vtkConstrainedSmoothingFilterRequestData .size For_threadNumber, 4 For_threadNumber: .zero 4 - .section.data.rel.ro.local,"aw" + .section .data.rel.ro.local._ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE,"awG",@progbits,_ZN26vtkStaticCellLinksTemplateIxE18ThreadedBuildLinksExxP12vtkCellArray,comdat .align 8 .LC0: .quad _ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE - .ident "GCC: (GNU) 14.0.1 20240127 (experimental)" + .ident "GCC: (GNU) 14.0.1 20240129 (experimental)" .section.note.GNU-stack,"",@progbits Jakub
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 8:03 AM Jakub Jelinek wrote: > > On Mon, Jan 29, 2024 at 06:36:47AM -0800, H.J. Lu wrote: > > TARGET_ASM_SELECT_RTX_SECTION is for constant in RTL. > > It should have a non-public label reference which can't be used > > by other TUs. The same section can contain other constants. > > If there is a COMAT issue, linker will catch it. > > Let me try to explain on short assembly snippet what I believe your patch is > doing and what I'm afraid of. I believe your patch when we need to emit > a RTL constant foo or foo+1 or foo+2 (where foo is defined in a comdat > section) instead of emitting using say foo in assembly puts those > constants into .data.rel.ro.local section determined by the decl that is > referenced. > Now, when first_tu.o wins and emits the qux comdat, it will contain > the .data.rel.ro.local.foo which bar function refers to, but in second_tu.o > it wants to refer to different offsets from the same function and loses. > > I simply believe the constants need to be in section based on what refers > to those symbols, not the value of those constants, and that is what we used > to do before your patch (and I'd like to understand what's wrong with what > GCC emits and why). > > first_tu.s: > > .section.text.foo,"axG",@progbits,qux,comdat > .p2align 4 > .type foo, @function > foo: > xorl%eax, %eax > ret > .size foo, .-foo > .text > .p2align 4 > .type bar, @function > bar: > movq.LC0(%rip), %xmm0 > ret > .size bar, .-bar > .section.data.rel.ro.local.foo,"awG",@progbits,qux,comdat > .align 8 > .LC0: > .quad foo > > second_tu.s: > > .section.text.foo,"axG",@progbits,qux,comdat > .p2align 4 > .type foo, @function > foo: > xorl%eax, %eax > ret > .size foo, .-foo > .text > .p2align 4 > .type baz, @function > baz: > movq.LC0(%rip), %xmm0 > ret I don't think this is valid. We can't reference a non-public symbol outside of a COMDAT group. It is OK to reference foo or foo + 1, but not .LC0. > .size baz, .-baz > .section.data.rel.ro.local.foo,"awG",@progbits,qux,comdat > .align 8 > .LC0: > .quad foo+1 > .text > .p2align 4 > .type corge, @function > corge: > movq.LC1(%rip), %xmm0 > ret > .size corge, .-corge > .section.data.rel.ro.local.foo,"awG",@progbits,qux,comdat > .align 8 > .LC1: > .quad foo+2 > gcc -shared -o test.so first_tu.s second_tu.s > `.data.rel.ro.local.foo' referenced in section `.text' of /tmp/cceeUWyH.o: > defined in discarded section `.data.rel.ro.local.foo[qux]' of /tmp/cceeUWyH.o > `.data.rel.ro.local.foo' referenced in section `.text' of /tmp/cceeUWyH.o: > defined in discarded section `.data.rel.ro.local.foo[qux]' of /tmp/cceeUWyH.o > collect2: error: ld returned 1 exit status > > Jakub > -- H.J.
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 06:36:47AM -0800, H.J. Lu wrote: > TARGET_ASM_SELECT_RTX_SECTION is for constant in RTL. > It should have a non-public label reference which can't be used > by other TUs. The same section can contain other constants. > If there is a COMAT issue, linker will catch it. Let me try to explain on short assembly snippet what I believe your patch is doing and what I'm afraid of. I believe your patch when we need to emit a RTL constant foo or foo+1 or foo+2 (where foo is defined in a comdat section) instead of emitting using say foo in assembly puts those constants into .data.rel.ro.local section determined by the decl that is referenced. Now, when first_tu.o wins and emits the qux comdat, it will contain the .data.rel.ro.local.foo which bar function refers to, but in second_tu.o it wants to refer to different offsets from the same function and loses. I simply believe the constants need to be in section based on what refers to those symbols, not the value of those constants, and that is what we used to do before your patch (and I'd like to understand what's wrong with what GCC emits and why). first_tu.s: .section.text.foo,"axG",@progbits,qux,comdat .p2align 4 .type foo, @function foo: xorl%eax, %eax ret .size foo, .-foo .text .p2align 4 .type bar, @function bar: movq.LC0(%rip), %xmm0 ret .size bar, .-bar .section.data.rel.ro.local.foo,"awG",@progbits,qux,comdat .align 8 .LC0: .quad foo second_tu.s: .section.text.foo,"axG",@progbits,qux,comdat .p2align 4 .type foo, @function foo: xorl%eax, %eax ret .size foo, .-foo .text .p2align 4 .type baz, @function baz: movq.LC0(%rip), %xmm0 ret .size baz, .-baz .section.data.rel.ro.local.foo,"awG",@progbits,qux,comdat .align 8 .LC0: .quad foo+1 .text .p2align 4 .type corge, @function corge: movq.LC1(%rip), %xmm0 ret .size corge, .-corge .section.data.rel.ro.local.foo,"awG",@progbits,qux,comdat .align 8 .LC1: .quad foo+2 gcc -shared -o test.so first_tu.s second_tu.s `.data.rel.ro.local.foo' referenced in section `.text' of /tmp/cceeUWyH.o: defined in discarded section `.data.rel.ro.local.foo[qux]' of /tmp/cceeUWyH.o `.data.rel.ro.local.foo' referenced in section `.text' of /tmp/cceeUWyH.o: defined in discarded section `.data.rel.ro.local.foo[qux]' of /tmp/cceeUWyH.o collect2: error: ld returned 1 exit status Jakub
Re: [PATCH] Handle function symbol reference in readonly data section
On Mon, Jan 29, 2024 at 3:03 AM Jakub Jelinek wrote:
>
> On Sat, Jan 27, 2024 at 07:10:55AM -0800, H.J. Lu wrote:
> > For function symbol reference in readonly data section, instead of putting
> > it in .data.rel.ro or .rodata.cst section, call function_rodata_section to
> > get the read-only or relocated read-only data section associated with the
> > function DECL so that the COMDAT section will be used for a COMDAT function
> > symbol.
>
> I have to admit I still don't understand what the linker doesn't like on
> what GCC emits and why references to the public symbols at the start of
> comdat sections are ok in .text but not in .data.rel.ro but are in .data
> or .rodata sections (or what the exact rules are, see also what we emit on
> __attribute__((noinline, noipa)) inline void foo () {}
> void bar () { foo (); } void (*p) () = foo; void (*const q) () = foo; void
> (*const *r) () =
> ).
> I've always thought that the problematic references are when something
> references non-public symbols in comdat sections, especially not at their
> start, because if linker selects some comdat section(s) from some other
> TU, there is no guarantee e.g. the code is identical (just in valid program
> should behave the same) and if such reference comes from other comdat that
> is kept or from non-comdat sections, the question is what should be
> referenced.
>
> But in this case, I believe we are referencing the function at the start of
> a code comdat section.
>
> Now, in my limited understanding what the patch does is totally wrong
> for multiple reasons. On the first testcase it changes
> - .section.data.rel.ro.local,"aw"
> + .section
> .data.rel.ro.local._ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE,"awG",@progbits,_ZN26vtkStaticCellLinksTemplateIxE18ThreadedBuildLinksExxP12vtkCellArray,comdat
> .align 8
> .LC0:
> .quad
> _ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE
> Now, I believe such a .data.rel.ro.local.* section is normally
> used for .data.rel.ro.local constants from the referenced function,
> if we have some relocatable constant needed in that function we
> emit those there.
> If linker picks up the comdat from current TU, it will be all fine,
> sure, but if it picks up the comdat from another TU, the
> .data.rel.ro.local._ZN4blah17_Function_handlerIFvvENS_5* section
> there might not be present or might contain some unrelated stuff.
> Given the handling of (const (plus (symbol_ref) (const_int)), we
> also don't know whether the section holds a reference to the start,
> or to some other offset of it, how many etc.
> And, we refenre a non-public symbol (.LC0) from non-comdat section
> to a comdat section.
TARGET_ASM_SELECT_RTX_SECTION is for constant in RTL.
It should have a non-public label reference which can't be used
by other TUs. The same section can contain other constants.
If there is a COMAT issue, linker will catch it.
> If I'm wrong on this, please try to explain.
>
> Jakub
>
--
H.J.
Re: [PATCH] Handle function symbol reference in readonly data section
On Sat, Jan 27, 2024 at 07:10:55AM -0800, H.J. Lu wrote:
> For function symbol reference in readonly data section, instead of putting
> it in .data.rel.ro or .rodata.cst section, call function_rodata_section to
> get the read-only or relocated read-only data section associated with the
> function DECL so that the COMDAT section will be used for a COMDAT function
> symbol.
I have to admit I still don't understand what the linker doesn't like on
what GCC emits and why references to the public symbols at the start of
comdat sections are ok in .text but not in .data.rel.ro but are in .data
or .rodata sections (or what the exact rules are, see also what we emit on
__attribute__((noinline, noipa)) inline void foo () {}
void bar () { foo (); } void (*p) () = foo; void (*const q) () = foo; void
(*const *r) () =
).
I've always thought that the problematic references are when something
references non-public symbols in comdat sections, especially not at their
start, because if linker selects some comdat section(s) from some other
TU, there is no guarantee e.g. the code is identical (just in valid program
should behave the same) and if such reference comes from other comdat that
is kept or from non-comdat sections, the question is what should be
referenced.
But in this case, I believe we are referencing the function at the start of
a code comdat section.
Now, in my limited understanding what the patch does is totally wrong
for multiple reasons. On the first testcase it changes
- .section.data.rel.ro.local,"aw"
+ .section
.data.rel.ro.local._ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE,"awG",@progbits,_ZN26vtkStaticCellLinksTemplateIxE18ThreadedBuildLinksExxP12vtkCellArray,comdat
.align 8
.LC0:
.quad
_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxx10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE
Now, I believe such a .data.rel.ro.local.* section is normally
used for .data.rel.ro.local constants from the referenced function,
if we have some relocatable constant needed in that function we
emit those there.
If linker picks up the comdat from current TU, it will be all fine,
sure, but if it picks up the comdat from another TU, the
.data.rel.ro.local._ZN4blah17_Function_handlerIFvvENS_5* section
there might not be present or might contain some unrelated stuff.
Given the handling of (const (plus (symbol_ref) (const_int)), we
also don't know whether the section holds a reference to the start,
or to some other offset of it, how many etc.
And, we refenre a non-public symbol (.LC0) from non-comdat section
to a comdat section.
If I'm wrong on this, please try to explain.
Jakub
[PATCH] Handle function symbol reference in readonly data section
For function symbol reference in readonly data section, instead of putting
it in .data.rel.ro or .rodata.cst section, call function_rodata_section to
get the read-only or relocated read-only data section associated with the
function DECL so that the COMDAT section will be used for a COMDAT function
symbol.
gcc/
PR rtl-optimization/113617
* varasm.cc (default_elf_select_rtx_section): Call
function_rodata_section to get the read-only or relocated
read-only data section for function symbol reference.
gcc/testsuite/
PR rtl-optimization/113617
* g++.dg/pr113617-1a.C: New test.
* g++.dg/pr113617-1b.C: Likewise.
---
gcc/testsuite/g++.dg/pr113617-1a.C | 170 +
gcc/testsuite/g++.dg/pr113617-1b.C | 8 ++
gcc/varasm.cc | 18 +++
3 files changed, 196 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/pr113617-1a.C
create mode 100644 gcc/testsuite/g++.dg/pr113617-1b.C
diff --git a/gcc/testsuite/g++.dg/pr113617-1a.C
b/gcc/testsuite/g++.dg/pr113617-1a.C
new file mode 100644
index 000..effd50841c0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr113617-1a.C
@@ -0,0 +1,170 @@
+// { dg-do compile { target fpic } }
+// { dg-require-visibility "" }
+// { dg-options "-O2 -std=c++11 -fPIC -fvisibility=hidden
-fvisibility-inlines-hidden" }
+
+namespace {
+template struct integral_constant {
+ static constexpr int value = __v;
+};
+template using __bool_constant = integral_constant<__v>;
+using true_type = __bool_constant;
+template struct __conditional {
+ template using type = _Tp;
+};
+template
+using __conditional_t = typename __conditional<_Cond>::type<_If, _Else>;
+true_type __trans_tmp_1;
+template struct remove_cv { using type = _Tp; };
+template
+struct __decay_selector
+: __conditional_t, _Up> {};
+template struct decay {
+ using type = typename __decay_selector<_Tp>::type;
+};
+}
+struct vtkCellArray {};
+namespace blah {
+struct _Any_data;
+enum _Manager_operation {};
+template class function;
+struct _Function_base {
+ using _Manager_type = bool (*)(_Any_data &, const _Any_data &,
+ _Manager_operation);
+ _Manager_type _M_manager;
+};
+template class _Function_handler;
+template
+struct _Function_handler<_Res(_ArgTypes...), _Functor> {
+ static bool _M_manager(_Any_data &, const _Any_data &, _Manager_operation) {
+return false;
+ }
+ __attribute__((noipa)) static _Res _M_invoke(const _Any_data &) {}
+};
+template
+struct function<_Res(_ArgTypes...)> : _Function_base {
+ template
+ using _Handler = _Function_handler<_Res(), _Functor>;
+ template function(_Functor) {
+using _My_handler = _Handler<_Functor>;
+_M_invoker = _My_handler::_M_invoke;
+_M_manager = _My_handler::_M_manager;
+ }
+ using _Invoker_type = _Res (*)(const _Any_data &);
+ _Invoker_type _M_invoker;
+};
+template class _Bind;
+template
+struct _Bind<_Functor(_Bound_args...)> {};
+template using __is_socketlike = decltype(__trans_tmp_1);
+template struct _Bind_helper {
+ typedef _Bind::type(
+ typename decay<_BoundArgs>::type...)>
+ type;
+};
+template
+__attribute__((noipa)) typename _Bind_helper<__is_socketlike<_Func>::value,
_Func, _BoundArgs...>::type
+bind(_Func, _BoundArgs...) { return typename
_Bind_helper<__is_socketlike<_Func>::value, _Func, _BoundArgs...>::type (); }
+template struct __uniq_ptr_impl {
+ template struct _Ptr { using type = _Up *; };
+ using pointer = typename _Ptr<_Tp>::type;
+};
+template struct unique_ptr {
+ using pointer = typename __uniq_ptr_impl<_Tp>::pointer;
+ pointer operator->();
+};
+}
+extern int For_threadNumber;
+namespace vtk {
+namespace detail {
+namespace smp {
+enum BackendType { Sequential, STDThread };
+template struct vtkSMPToolsImpl {
+ template
+ __attribute__((noipa)) void For(long long, long long, long long,
FunctorInternal &) {}
+};
+struct vtkSMPThreadPool {
+ vtkSMPThreadPool(int);
+ void DoJob(blah::function);
+};
+template
+__attribute__((noipa)) void ExecuteFunctorSTDThread(void *, long long, long
long, long long) {}
+template <>
+template
+void vtkSMPToolsImpl::For(long long, long long last, long long
grain,
+ FunctorInternal ) {
+ vtkSMPThreadPool pool(For_threadNumber);
+ for (;;) {
+auto job = blah::bind(ExecuteFunctorSTDThread, , grain,
+ grain, last);
+pool.DoJob(job);
+ }
+}
+struct vtkSMPToolsAPI {
+ static vtkSMPToolsAPI ();
+ template
+ void For(long first, long last, long grain, FunctorInternal fi) {
+switch (ActivatedBackend) {
+case Sequential:
+ SequentialBackend->For(first, last, grain, fi);
+case STDThread:
+ STDThreadBackend->For(first, last, grain, fi);
+}
+ }
+ BackendType ActivatedBackend;
+ blah::unique_ptr> SequentialBackend;
+ blah::unique_ptr> STDThreadBackend;
+};
+template struct vtkSMPTools_FunctorInternal;
+template struct
