This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]