> On Mon, Feb 26, 2018 at 8:54 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
> >> On Mon, Feb 26, 2018 at 7:47 AM, Jan Hubicka <hubi...@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.to...@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?
OK,
Honza
>
> --
> H.J.
