| Message ID | tencent_0C568854E3071FB0A0A4F69F23022E578C06@qq.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,aarch64] Fix function multiversioning dispatcher link error with LTO | expand |
Yangyu Chen <cyy@cyyself.name> writes: > We forgot to apply DECL_EXTERNAL to __init_cpu_features_resolver decl. When > building with LTO, the linker cannot find the > __init_cpu_features_resolver.lto_priv* symbol, causing the link error. > > This patch gets this fixed by adding DECL_EXTERNAL to the decl. To avoid used > but never defined warning for this symbol, we also mark TREE_PUBLIC to the decl. > We should also mark the decl having hidden visibility. And fix the attribute in > the same way for __aarch64_cpu_features identifier. > > Minimal steps to reproduce the bug: > > echo '__attribute__((target_clones("default", "aes"))) void func1() { }' > 1.c > echo '__attribute__((target_clones("default", "aes"))) void func2() { }' > 2.c > echo 'void func1();void func2();int main(){func1();func2();return 0;}' > main.c > gcc -flto -c 1.c 2.c > gcc -flto main.c 1.o 2.o > > Fixes: 0cfde688e213 ("[aarch64] Add function multiversioning support") > > gcc/ChangeLog: > > * config/aarch64/aarch64.cc (dispatch_function_versions): Adding > DECL_EXTERNAL, TREE_PUBLIC and hidden DECL_VISIBILITY to > __init_cpu_features_resolver and __aarch64_cpu_features. Thanks, LGTM. I've tested this locally and was about to push, but then realised: since you've already contributed changes (great!), it probably wouldn't be acceptable to treat it as trivial for copyright purposes. Could you confirm that you're contributing under the DCO: https://gcc.gnu.org/dco.html ? If so, could you repost with a Signed-off-by? Sorry for the administrivia. Richard > --- > gcc/config/aarch64/aarch64.cc | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index 5770491b30c..2b2d5b9e390 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -20437,6 +20437,10 @@ dispatch_function_versions (tree dispatch_decl, > tree init_fn_id = get_identifier ("__init_cpu_features_resolver"); > tree init_fn_decl = build_decl (UNKNOWN_LOCATION, FUNCTION_DECL, > init_fn_id, init_fn_type); > + DECL_EXTERNAL (init_fn_decl) = 1; > + TREE_PUBLIC (init_fn_decl) = 1; > + DECL_VISIBILITY (init_fn_decl) = VISIBILITY_HIDDEN; > + DECL_VISIBILITY_SPECIFIED (init_fn_decl) = 1; > tree arg1 = DECL_ARGUMENTS (dispatch_decl); > tree arg2 = TREE_CHAIN (arg1); > ifunc_cpu_init_stmt = gimple_build_call (init_fn_decl, 2, arg1, arg2); > @@ -20456,6 +20460,9 @@ dispatch_function_versions (tree dispatch_decl, > get_identifier ("__aarch64_cpu_features"), > global_type); > DECL_EXTERNAL (global_var) = 1; > + TREE_PUBLIC (global_var) = 1; > + DECL_VISIBILITY (global_var) = VISIBILITY_HIDDEN; > + DECL_VISIBILITY_SPECIFIED (global_var) = 1; > tree mask_var = create_tmp_var (long_long_unsigned_type_node); > > tree component_expr = build3 (COMPONENT_REF, long_long_unsigned_type_node,
> On Oct 30, 2024, at 19:59, Richard Sandiford <richard.sandiford@arm.com> wrote: > > Yangyu Chen <cyy@cyyself.name> writes: >> We forgot to apply DECL_EXTERNAL to __init_cpu_features_resolver decl. When >> building with LTO, the linker cannot find the >> __init_cpu_features_resolver.lto_priv* symbol, causing the link error. >> >> This patch gets this fixed by adding DECL_EXTERNAL to the decl. To avoid used >> but never defined warning for this symbol, we also mark TREE_PUBLIC to the decl. >> We should also mark the decl having hidden visibility. And fix the attribute in >> the same way for __aarch64_cpu_features identifier. >> >> Minimal steps to reproduce the bug: >> >> echo '__attribute__((target_clones("default", "aes"))) void func1() { }' > 1.c >> echo '__attribute__((target_clones("default", "aes"))) void func2() { }' > 2.c >> echo 'void func1();void func2();int main(){func1();func2();return 0;}' > main.c >> gcc -flto -c 1.c 2.c >> gcc -flto main.c 1.o 2.o >> >> Fixes: 0cfde688e213 ("[aarch64] Add function multiversioning support") >> >> gcc/ChangeLog: >> >> * config/aarch64/aarch64.cc (dispatch_function_versions): Adding >> DECL_EXTERNAL, TREE_PUBLIC and hidden DECL_VISIBILITY to >> __init_cpu_features_resolver and __aarch64_cpu_features. > > Thanks, LGTM. I've tested this locally and was about to push, but then > realised: since you've already contributed changes (great!), it probably > wouldn't be acceptable to treat it as trivial for copyright purposes. > Could you confirm that you're contributing under the DCO: > https://gcc.gnu.org/dco.html ? If so, could you repost with a > Signed-off-by? > > Sorry for the administrivia. I added signed-off-by and revised to v4: https://patchwork.sourceware.org/project/gcc/patch/tencent_F08BE088F6B1E3152E508C63C870E31CDF06@qq.com/ Since this is a fix patch, it should also be committed to the releases/gcc-14 branch. Thanks, Yangyu Chen > > Richard > >> --- >> gcc/config/aarch64/aarch64.cc | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc >> index 5770491b30c..2b2d5b9e390 100644 >> --- a/gcc/config/aarch64/aarch64.cc >> +++ b/gcc/config/aarch64/aarch64.cc >> @@ -20437,6 +20437,10 @@ dispatch_function_versions (tree dispatch_decl, >> tree init_fn_id = get_identifier ("__init_cpu_features_resolver"); >> tree init_fn_decl = build_decl (UNKNOWN_LOCATION, FUNCTION_DECL, >> init_fn_id, init_fn_type); >> + DECL_EXTERNAL (init_fn_decl) = 1; >> + TREE_PUBLIC (init_fn_decl) = 1; >> + DECL_VISIBILITY (init_fn_decl) = VISIBILITY_HIDDEN; >> + DECL_VISIBILITY_SPECIFIED (init_fn_decl) = 1; >> tree arg1 = DECL_ARGUMENTS (dispatch_decl); >> tree arg2 = TREE_CHAIN (arg1); >> ifunc_cpu_init_stmt = gimple_build_call (init_fn_decl, 2, arg1, arg2); >> @@ -20456,6 +20460,9 @@ dispatch_function_versions (tree dispatch_decl, >> get_identifier ("__aarch64_cpu_features"), >> global_type); >> DECL_EXTERNAL (global_var) = 1; >> + TREE_PUBLIC (global_var) = 1; >> + DECL_VISIBILITY (global_var) = VISIBILITY_HIDDEN; >> + DECL_VISIBILITY_SPECIFIED (global_var) = 1; >> tree mask_var = create_tmp_var (long_long_unsigned_type_node); >> >> tree component_expr = build3 (COMPONENT_REF, long_long_unsigned_type_node,
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc index 5770491b30c..2b2d5b9e390 100644 --- a/gcc/config/aarch64/aarch64.cc +++ b/gcc/config/aarch64/aarch64.cc @@ -20437,6 +20437,10 @@ dispatch_function_versions (tree dispatch_decl, tree init_fn_id = get_identifier ("__init_cpu_features_resolver"); tree init_fn_decl = build_decl (UNKNOWN_LOCATION, FUNCTION_DECL, init_fn_id, init_fn_type); + DECL_EXTERNAL (init_fn_decl) = 1; + TREE_PUBLIC (init_fn_decl) = 1; + DECL_VISIBILITY (init_fn_decl) = VISIBILITY_HIDDEN; + DECL_VISIBILITY_SPECIFIED (init_fn_decl) = 1; tree arg1 = DECL_ARGUMENTS (dispatch_decl); tree arg2 = TREE_CHAIN (arg1); ifunc_cpu_init_stmt = gimple_build_call (init_fn_decl, 2, arg1, arg2); @@ -20456,6 +20460,9 @@ dispatch_function_versions (tree dispatch_decl, get_identifier ("__aarch64_cpu_features"), global_type); DECL_EXTERNAL (global_var) = 1; + TREE_PUBLIC (global_var) = 1; + DECL_VISIBILITY (global_var) = VISIBILITY_HIDDEN; + DECL_VISIBILITY_SPECIFIED (global_var) = 1; tree mask_var = create_tmp_var (long_long_unsigned_type_node); tree component_expr = build3 (COMPONENT_REF, long_long_unsigned_type_node,
We forgot to apply DECL_EXTERNAL to __init_cpu_features_resolver decl. When building with LTO, the linker cannot find the __init_cpu_features_resolver.lto_priv* symbol, causing the link error. This patch gets this fixed by adding DECL_EXTERNAL to the decl. To avoid used but never defined warning for this symbol, we also mark TREE_PUBLIC to the decl. We should also mark the decl having hidden visibility. And fix the attribute in the same way for __aarch64_cpu_features identifier. Minimal steps to reproduce the bug: echo '__attribute__((target_clones("default", "aes"))) void func1() { }' > 1.c echo '__attribute__((target_clones("default", "aes"))) void func2() { }' > 2.c echo 'void func1();void func2();int main(){func1();func2();return 0;}' > main.c gcc -flto -c 1.c 2.c gcc -flto main.c 1.o 2.o Fixes: 0cfde688e213 ("[aarch64] Add function multiversioning support") gcc/ChangeLog: * config/aarch64/aarch64.cc (dispatch_function_versions): Adding DECL_EXTERNAL, TREE_PUBLIC and hidden DECL_VISIBILITY to __init_cpu_features_resolver and __aarch64_cpu_features. --- gcc/config/aarch64/aarch64.cc | 7 +++++++ 1 file changed, 7 insertions(+)