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? -- H.J.