Re: [PATCH] i386: Update -mfunction-return= for return with pop

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, Feb 26, 2018 at 7:44 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Mon, Feb 26, 2018 at 7:06 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >> On Mon, Feb 26, 2018 at 2:58 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >> >> On Mon, Feb 26, 2018 at 2:05 AM, Martin Liška <mliska@suse.cz> wrote:
>> >> >> > On 02/25/2018 02:37 AM, H.J. Lu wrote:
>> >> >> >>       PR target/r84530
>> >> >> >
>> >> >> > Just a nit I've noticed:
>> >> >> >
>> >> >> > s/r84530/84530
>> >> >>
>> >> >> Fixed.
>> >> >>
>> >> >> --
>> >> >> H.J.
>> >> >
>> >> >> From f08b8721ed038cefcee5a0bb1329b90a2d322269 Mon Sep 17 00:00:00 2001
>> >> >> From: "H.J. Lu" <hjl.tools@gmail.com>
>> >> >> Date: Sat, 24 Feb 2018 05:05:49 -0800
>> >> >> Subject: [PATCH] i386: Update -mfunction-return= for return with pop
>> >> >>
>> >> >> When -mfunction-return= is used, simple_return_pop_internal should pop
>> >> >> return address into ECX register, adjust stack by bytes to pop from stack
>> >> >> and jump to the return thunk via ECX register.
>> >> >>
>> >> >> Tested on i686 and x86-64.
>> >> >>
>> >> >>       PR target/84530
>> >> >>       * config/i386/i386-protos.h (ix86_output_indirect_jmp): Remove
>> >> >>       the bool argument.
>> >> >>       (ix86_output_indirect_function_return): New prototype.
>> >> >>       (ix86_split_simple_return_pop_internal): Likewise.
>> >> >>       * config/i386/i386.c (indirect_return_via_cx): New.
>> >> >>       (indirect_return_via_cx_bnd): Likewise.
>> >> >>       (indirect_thunk_name): Handle return va CX_REG.
>> >> >>       (output_indirect_thunk_function): Create alias for
>> >> >>       __x86_return_thunk_[re]cx and __x86_return_thunk_[re]cx_bnd.
>> >> >>       (ix86_output_indirect_jmp): Remove the bool argument.
>> >> >>       (ix86_output_indirect_function_return): New function.
>> >> >>       (ix86_split_simple_return_pop_internal): Likewise.
>> >> >>       * config/i386/i386.md (*indirect_jump): Don't pass false
>> >> >>       to ix86_output_indirect_jmp.
>> >> >>       (*tablejump_1): Likewise.
>> >> >>       (simple_return_pop_internal): Change it to define_insn_and_split.
>> >> >>       Call ix86_split_simple_return_pop_internal to split it for
>> >> >>       -mfunction-return=.
>> >> >>       (simple_return_indirect_internal): Call
>> >> >>       ix86_output_indirect_function_return instead of
>> >> >>       ix86_output_indirect_jmp.
>> >> >
>> >> > It seems to make sense. Since LLVM has picked up our thunk names now,
>> >> > I wonder if they have same bug or someohow already assigned a name
>> >> > to this thunk. In the second case we probably ought to match it. Do
>> >> > you know what LLVM uses?
>> >> >
>> >>
>> >> I don't think LLVM has implemented -mfunction-return=.   I disclosed
>> >> my -mindirect-branch= approach to LLVM last year.  At the time, they
>> >> wanted freedom in their implementation and didn't want to be compatible
>> >> with mine.   Later they added GCC compatibility so that LLVM may be
>> >> used to compile Linux kernel.  I believe it will be the same case if this
>> >> option is used to compile Linux kernel.
>> >
>> > Patch is OK then, thanks!
>> >
>>
>> Checked into trunk.  OK for backport to GCC 7 branch after a few days?
> OK,

I checked in this patch to update comments.  I will backport it together to
GCC 7 branch.

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c (revision 258004)
+++ config/i386/i386.c (working copy)
@@ -28999,8 +28999,7 @@ ix86_output_indirect_branch (rtx call_op
     ix86_output_indirect_branch_via_push (call_op, xasm, sibcall_p);
 }

-/* Output indirect jump.  CALL_OP is the jump target.  Jump is a
-   function return if RET_P is true.  */
+/* Output indirect jump.  CALL_OP is the jump target.  */

 const char *
 ix86_output_indirect_jmp (rtx call_op)


-- 
H.J.