This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PING: [PATCH] i386: Add TARGET_INDIRECT_BRANCH_REGISTER
On Mon, Feb 26, 2018 at 8:54 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Mon, Feb 26, 2018 at 7:47 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > Hi,
>> > my main concern about the patch is that we now have -mindirect-branch=thunk-extern
>> > which is intended to work well and is used by kernel, but we also have other modes
>> > that are documented and as such they should work but they may lead to invalid
>> > unwind info (or did I miss anything imporant here?).
>> > Why we can't fix the others as well?
>> >
>>
>> I took a closer look at my commit message. It does leave an impression that
>> only -mindirect-branch=thunk-extern is fixed. But in fact, all
>> -mindirect-branch=
>> choices are fixed.
>
> I see, sorry for confussion!
>>
>> 1. -mindirect-branch=thunk generates:
>>
>> .cfi_startproc
>> pushq %rbx
>> .cfi_def_cfa_offset 16
>> .cfi_offset 3, -16
>> movq (%rdi), %rax
>> movq %rdi, %rbx
>> movq 16(%rax), %rax
>> call __x86_indirect_thunk_rax
>> movq (%rbx), %rax
>> movq %rbx, %rdi
>> popq %rbx
>> .cfi_def_cfa_offset 8
>> movq 16(%rax), %rax
>> jmp __x86_indirect_thunk_rax
>> .cfi_endproc
>>
>> 2. -mindirect-branch=thunk-inline generates:
>>
>> .cfi_startproc
>> pushq %rbx
>> .cfi_def_cfa_offset 16
>> .cfi_offset 3, -16
>> movq (%rdi), %rax
>> movq %rdi, %rbx
>> movq 16(%rax), %rax
>> jmp .LIND1
>> .LIND0:
>> call .LIND3
>> .LIND2:
>> pause
>> lfence
>> jmp .LIND2
>> .LIND3:
>> mov %rax, (%rsp)
>> ret
>> .LIND1:
>> call .LIND0
>> movq (%rbx), %rax
>> movq %rbx, %rdi
>> popq %rbx
>> .cfi_def_cfa_offset 8
>> movq 16(%rax), %rax
>> call .LIND5
>> .LIND4:
>> pause
>> lfence
>> jmp .LIND4
>> .LIND5:
>> mov %rax, (%rsp)
>> ret
>> .cfi_endproc
>>
>> I updated the commit message with
>>
>> This patch adds TARGET_INDIRECT_BRANCH_REGISTER to force indirect
>> branch via register whenever -mindirect-branch= is used.
>>
>> OK for trunk?
>>
>> Thanks.
>>
>> --
>> H.J.
>
>> From bd0672bd070da6fa4ff630540c1d87df3e8fdd53 Mon Sep 17 00:00:00 2001
>> From: "H.J. Lu" <hjl.tools@gmail.com>
>> Date: Fri, 26 Jan 2018 15:54:25 -0800
>> Subject: [PATCH] i386: Add TARGET_INDIRECT_BRANCH_REGISTER
>>
>> For
>>
>> ---
>> struct C {
>> virtual ~C();
>> virtual void f();
>> };
>>
>> void
>> f (C *p)
>> {
>> p->f();
>> p->f();
>> }
>> ---
>>
>> -mindirect-branch=thunk-extern -O2 on x86-64 GNU/Linux generates:
>>
>> _Z1fP1C:
>> .LFB0:
>> .cfi_startproc
>> pushq %rbx
>> .cfi_def_cfa_offset 16
>> .cfi_offset 3, -16
>> movq (%rdi), %rax
>> movq %rdi, %rbx
>> jmp .LIND1
>> .LIND0:
>> pushq 16(%rax)
>> jmp __x86_indirect_thunk
>> .LIND1:
>> call .LIND0
>> movq (%rbx), %rax
>> movq %rbx, %rdi
>> popq %rbx
>> .cfi_def_cfa_offset 8
>> movq 16(%rax), %rax
>> jmp __x86_indirect_thunk_rax
>> .cfi_endproc
>>
>> x86-64 is supposed to have asynchronous unwind tables by default, but
>> there is nothing that reflects the change in the (relative) frame
>> address after .LIND0. That region really has to be moved outside of
>> the .cfi_startproc/.cfi_endproc bracket.
>>
>> This patch adds TARGET_INDIRECT_BRANCH_REGISTER to force indirect
>> branch via register whenever -mindirect-branch= is used. Now,
>> -mindirect-branch=thunk-extern -O2 on x86-64 GNU/Linux generates:
>>
>> _Z1fP1C:
>> .LFB0:
>> .cfi_startproc
>> pushq %rbx
>> .cfi_def_cfa_offset 16
>> .cfi_offset 3, -16
>> movq (%rdi), %rax
>> movq %rdi, %rbx
>> movq 16(%rax), %rax
>> call __x86_indirect_thunk_rax
>> movq (%rbx), %rax
>> movq %rbx, %rdi
>> popq %rbx
>> .cfi_def_cfa_offset 8
>> movq 16(%rax), %rax
>> jmp __x86_indirect_thunk_rax
>> .cfi_endproc
>>
>> so that "-mindirect-branch=thunk-extern" is equivalent to
>> "-mindirect-branch=thunk-extern -mindirect-branch-register", which is
>> used by Linux kernel.
>>
>> gcc/
>>
>> PR target/84039
>> * config/i386/constraints.md (Bs): Replace
>> ix86_indirect_branch_register with
>> TARGET_INDIRECT_BRANCH_REGISTER.
>> (Bw): Likewise.
>> * config/i386/i386.md (indirect_jump): Likewise.
>> (tablejump): Likewise.
>> (*sibcall_memory): Likewise.
>> (*sibcall_value_memory): Likewise.
>> Peepholes of indirect call and jump via memory: Likewise.
>> (*sibcall_GOT_32): Disallowed for TARGET_INDIRECT_BRANCH_REGISTER.
>> (*sibcall_value_GOT_32): Likewise.
>> * config/i386/i386.opt: Likewise.
>> * config/i386/predicates.md (indirect_branch_operand): Likewise.
>> (GOT_memory_operand): Likewise.
>> (call_insn_operand): Likewise.
>> (sibcall_insn_operand): Likewise.
>> (GOT32_symbol_operand): Likewise.
>> * config/i386/i386.h (TARGET_INDIRECT_BRANCH_REGISTER): New.
>
> OK,
> thanks!
> Honza
OK for backport to GCC 7 after a few days?
--
H.J.