On Mon, Jan 20, 2020 at 2:25 AM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Fri, Jan 17, 2020 at 10:25 AM Martin Liška <mli...@suse.cz> wrote:
> >
> > Hi.
> >
> > The patch removes need to have a gnu_indirect_function global
> > symbol. That aligns the code with what ppc64 target does.
> >
> > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >
> > Ready to be installed?
>
> Did you verify the result actually works? I'm not sure we have any runtime
> test
> coverage for the feature and non-public functions and you don't add a testcase
> either. Maybe there's interesting coverage in the binutils or glibc testsuite
> (though both might not use the compilers ifunc feature...).
>
> The patch also suspiciously lacks removal of actually making the resolver
> TREE_PUBLIC if the default implementation was not ... so I wonder whether
> you verified that the resolver _is_ indeed local.
>
> HJ, do you know anything about this requirement? It's that way since
> the original contribution of multi-versioning by Google...
We can that only if function is static:
[hjl@gnu-cfl-2 tmp]$ cat x.c
__attribute__((target_clones("avx","default")))
int
foo ()
{
return -2;
}
[hjl@gnu-cfl-2 tmp]$ gcc -S -O2 x.c
[hjl@gnu-cfl-2 tmp]$ cat x.s
.file "x.c"
.text
.p2align 4
.type foo.default.1, @function
foo.default.1:
.LFB0:
.cfi_startproc
movl $-2, %eax
ret
.cfi_endproc
.LFE0:
.size foo.default.1, .-foo.default.1
.p2align 4
.type foo.avx.0, @function
foo.avx.0:
.LFB1:
.cfi_startproc
movl $-2, %eax
ret
.cfi_endproc
.LFE1:
.size foo.avx.0, .-foo.avx.0
.section .text.foo.resolver,"axG",@progbits,foo.resolver,comdat
.p2align 4
.weak foo.resolver
.type foo.resolver, @function
foo.resolver:
.LFB3:
.cfi_startproc
subq $8, %rsp
.cfi_def_cfa_offset 16
call __cpu_indicator_init
movl $foo.default.1, %eax
movl $foo.avx.0, %edx
testb $2, __cpu_model+13(%rip)
cmovne %rdx, %rax
addq $8, %rsp
.cfi_def_cfa_offset 8
ret
.cfi_endproc
.LFE3:
.size foo.resolver, .-foo.resolver
.globl foo
.type foo, @gnu_indirect_function
.set foo,foo.resolver
.ident "GCC: (GNU) 9.2.1 20191120 (Red Hat 9.2.1-2)"
.section .note.GNU-stack,"",@progbits
[hjl@gnu-cfl-2 tmp]$
In this case, foo must be global.
> Richard.
>
> > Thanks,
> > Martin
> >
> > gcc/ChangeLog:
> >
> > 2020-01-17 Martin Liska <mli...@suse.cz>
> >
> > PR target/93274
> > * config/i386/i386-features.c (make_resolver_func):
> > Align the code with ppc64 target implementaion.
> > We do not need to have gnu_indirect_function
> > as a global function.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2020-01-17 Martin Liska <mli...@suse.cz>
> >
> > PR target/93274
> > * gcc.target/i386/pr81213.c: Adjust to not expect
> > a global unique name.
> > ---
> > gcc/config/i386/i386-features.c | 20 +++++---------------
> > gcc/testsuite/gcc.target/i386/pr81213.c | 4 ++--
> > 2 files changed, 7 insertions(+), 17 deletions(-)
> >
> >
--
H.J.
