i386: Use __glibc_likely/__glibc_likely in dl-machine.h
Commit Message
The differences in elf/dl-reloc.os are
Comments
On Mon, Feb 05, 2018 at 04:28:07AM -0800, H.J. Lu wrote:
> The differences in elf/dl-reloc.os are
>
> --- before 2018-02-05 03:53:31.970492246 -0800
> +++ after 2018-02-05 03:53:49.719902340 -0800
> @@ -1202,9 +1202,9 @@ _dl_relocate_object:
> movl -60(%ebp), %eax
> testl %eax, %eax
> je .L249
> - movl 8(%eax), %eax
> - movl 8(%ebx), %esi
> - cmpl %esi, %eax
> + movl 8(%eax), %esi
> + movl 8(%ebx), %eax
> + cmpl %eax, %esi
> ja .L284
> jb .L707
> .L285:
> @@ -2255,7 +2255,7 @@ _dl_relocate_object:
> cmpl $6, %edi
> movl $4, %edx
> je .L132
> - cmpl %ecx, %eax
> + cmpl %eax, %ecx
> je .L350
> cmpl $7, %edi
> je .L419
> @@ -2735,7 +2735,7 @@ _dl_relocate_object:
> je .L120
> .L121:
> movl -96(%ebp), %edx
> - movl $640, 8(%esp)
> + movl $639, 8(%esp)
> leal __PRETTY_FUNCTION__.9431@GOTOFF(%edx), %eax
> movl %eax, 12(%esp)
> leal .LC9@GOTOFF(%edx), %eax
> @@ -3454,10 +3454,10 @@ _dl_relocate_object:
> movl -152(%ebp), %eax
> movl %eax, 4(%esp)
> call _dl_dprintf
> - movl -60(%ebp), %eax
> - movl 8(%ebx), %esi
> + movl 8(%ebx), %eax
> + movl -60(%ebp), %ebx
> movl -112(%ebp), %edx
> - movl 8(%eax), %eax
> + movl 8(%ebx), %esi
> jmp .L285
> .L713:
> movl %esi, (%esp)
>
> I will check it in shortly.
>
> H.J.
> ---
> * sysdeps/i386/dl-machine.h (elf_machine_rel): Replace
> __builtin_expect with __glibc_likely and __glibc_likely.
> (elf_machine_rela): Likewise.
> (elf_machine_lazy_rel): Likewise.
Why this patch has been committed without review?
I haven't looked into the change itself, but the commit message and
the changelog entry are very confusing because of
"__glibc_likely/__glibc_likely" and "__glibc_likely and __glibc_likely".
On Mon, Feb 5, 2018 at 7:28 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> The differences in elf/dl-reloc.os are
>
> --- before 2018-02-05 03:53:31.970492246 -0800
> +++ after 2018-02-05 03:53:49.719902340 -0800
> @@ -1202,9 +1202,9 @@ _dl_relocate_object:
> movl -60(%ebp), %eax
> testl %eax, %eax
> je .L249
> - movl 8(%eax), %eax
> - movl 8(%ebx), %esi
> - cmpl %esi, %eax
> + movl 8(%eax), %esi
> + movl 8(%ebx), %eax
> + cmpl %eax, %esi
Since __glibc_(un)likely just expand back to __builtin_expect, why are
there any assembly differences at all?
On Mon, Feb 5, 2018 at 12:23 PM, Zack Weinberg <zackw@panix.com> wrote:
> On Mon, Feb 5, 2018 at 7:28 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> The differences in elf/dl-reloc.os are
>>
>> --- before 2018-02-05 03:53:31.970492246 -0800
>> +++ after 2018-02-05 03:53:49.719902340 -0800
>> @@ -1202,9 +1202,9 @@ _dl_relocate_object:
>> movl -60(%ebp), %eax
>> testl %eax, %eax
>> je .L249
>> - movl 8(%eax), %eax
>> - movl 8(%ebx), %esi
>> - cmpl %esi, %eax
>> + movl 8(%eax), %esi
>> + movl 8(%ebx), %eax
>> + cmpl %eax, %esi
>
> Since __glibc_(un)likely just expand back to __builtin_expect, why are
> there any assembly differences at all?
These changes are trigger by
@ -663,7 +662,8 @@ elf_machine_lazy_rel (struct link_map *map,
/* Check for unexpected PLT reloc type. */
if (__glibc_likely (r_type == R_386_JMP_SLOT))
{
- if (__builtin_expect (map->l_mach.plt, 0) == 0)
+ /* Prelink has been deprecated. */
+ if (__glibc_likely (map->l_mach.plt == 0))
*reloc_addr += l_addr;
else
*reloc_addr = (map->l_mach.plt
Although they are equivalent, GCC treats
if (__builtin_expect (map->l_mach.plt, 0) == 0)
slightly differently from
if (__glibc_likely (map->l_mach.plt == 0))
such that the outputs aren't identical.
On Mon, Feb 5, 2018 at 3:52 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> Although they are equivalent, GCC treats
>
> if (__builtin_expect (map->l_mach.plt, 0) == 0)
>
> slightly differently from
>
> if (__glibc_likely (map->l_mach.plt == 0))
>
> such that the outputs aren't identical.
Thanks for the clarification.
zw
@@ -1202,9 +1202,9 @@ _dl_relocate_object:
movl -60(%ebp), %eax
testl %eax, %eax
je .L249
- movl 8(%eax), %eax
- movl 8(%ebx), %esi
- cmpl %esi, %eax
+ movl 8(%eax), %esi
+ movl 8(%ebx), %eax
+ cmpl %eax, %esi
ja .L284
jb .L707
.L285:
@@ -2255,7 +2255,7 @@ _dl_relocate_object:
cmpl $6, %edi
movl $4, %edx
je .L132
- cmpl %ecx, %eax
+ cmpl %eax, %ecx
je .L350
cmpl $7, %edi
je .L419
@@ -2735,7 +2735,7 @@ _dl_relocate_object:
je .L120
.L121:
movl -96(%ebp), %edx
- movl $640, 8(%esp)
+ movl $639, 8(%esp)
leal __PRETTY_FUNCTION__.9431@GOTOFF(%edx), %eax
movl %eax, 12(%esp)
leal .LC9@GOTOFF(%edx), %eax
@@ -3454,10 +3454,10 @@ _dl_relocate_object:
movl -152(%ebp), %eax
movl %eax, 4(%esp)
call _dl_dprintf
- movl -60(%ebp), %eax
- movl 8(%ebx), %esi
+ movl 8(%ebx), %eax
+ movl -60(%ebp), %ebx
movl -112(%ebp), %edx
- movl 8(%eax), %eax
+ movl 8(%ebx), %esi
jmp .L285
.L713:
movl %esi, (%esp)
I will check it in shortly.
H.J.
---
* sysdeps/i386/dl-machine.h (elf_machine_rel): Replace
__builtin_expect with __glibc_likely and __glibc_likely.
(elf_machine_rela): Likewise.
(elf_machine_lazy_rel): Likewise.
---
sysdeps/i386/dl-machine.h | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
@@ -323,10 +323,9 @@ elf_machine_rel (struct link_map *map, const Elf32_Rel *reloc,
Elf32_Addr value = sym_map == NULL ? 0 : sym_map->l_addr + sym->st_value;
if (sym != NULL
- && __builtin_expect (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC,
- 0)
- && __builtin_expect (sym->st_shndx != SHN_UNDEF, 1)
- && __builtin_expect (!skip_ifunc, 1))
+ && __glibc_unlikely (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC)
+ && __glibc_likely (sym->st_shndx != SHN_UNDEF)
+ && __glibc_likely (!skip_ifunc))
{
# ifndef RTLD_BOOTSTRAP
if (sym_map != map
@@ -456,8 +455,8 @@ elf_machine_rel (struct link_map *map, const Elf32_Rel *reloc,
/* This can happen in trace mode if an object could not be
found. */
break;
- if (__builtin_expect (sym->st_size > refsym->st_size, 0)
- || (__builtin_expect (sym->st_size < refsym->st_size, 0)
+ if (__glibc_unlikely (sym->st_size > refsym->st_size)
+ || (__glibc_unlikely(sym->st_size < refsym->st_size)
&& GLRO(dl_verbose)))
{
const char *strtab;
@@ -504,9 +503,9 @@ elf_machine_rela (struct link_map *map, const Elf32_Rela *reloc,
Elf32_Addr value = sym == NULL ? 0 : sym_map->l_addr + sym->st_value;
if (sym != NULL
- && __builtin_expect (sym->st_shndx != SHN_UNDEF, 1)
- && __builtin_expect (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC, 0)
- && __builtin_expect (!skip_ifunc, 1))
+ && __glibc_likely (sym->st_shndx != SHN_UNDEF)
+ && __glibc_unlikely (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC)
+ && __glibc_likely (!skip_ifunc))
value = ((Elf32_Addr (*) (void)) value) ();
switch (ELF32_R_TYPE (reloc->r_info))
@@ -601,8 +600,8 @@ elf_machine_rela (struct link_map *map, const Elf32_Rela *reloc,
/* This can happen in trace mode if an object could not be
found. */
break;
- if (__builtin_expect (sym->st_size > refsym->st_size, 0)
- || (__builtin_expect (sym->st_size < refsym->st_size, 0)
+ if (__glibc_unlikely (sym->st_size > refsym->st_size)
+ || (__glibc_unlikely (sym->st_size < refsym->st_size)
&& GLRO(dl_verbose)))
{
const char *strtab;
@@ -663,7 +662,8 @@ elf_machine_lazy_rel (struct link_map *map,
/* Check for unexpected PLT reloc type. */
if (__glibc_likely (r_type == R_386_JMP_SLOT))
{
- if (__builtin_expect (map->l_mach.plt, 0) == 0)
+ /* Prelinked has been deprecated. */
+ if (__glibc_likely (map->l_mach.plt == 0))
*reloc_addr += l_addr;
else
*reloc_addr = (map->l_mach.plt