From: Uros Bizjak <ubizjak@gmail.com>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] PR target/68937: i686: -fno-plt produces wrong code (maybe only with tailcall
Date: Thu, 17 Dec 2015 10:04:00 -0000 [thread overview]
Message-ID: <CAFULd4YMDKgyWQDiE5+e1WDCJad3tiyOztVAPb4TnktVdCvqUA@mail.gmail.com> (raw)
In-Reply-To: <20151216232951.GA17976@intel.com>
On Thu, Dec 17, 2015 at 12:29 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> Since sibcall never returns, we can only use call-clobbered register
> as GOT base. Otherwise, callee-saved register used as GOT base won't
> be properly restored.
>
> Tested on x86-64 with -m32. OK for trunk?
You don't have to add explicit clobber for members of "CLOBBERED_REGS"
class, and register_no_elim_operand predicate should be used with "U"
constraint. Also, please introduce new predicate, similar to how
GOT_memory_operand is defined and handled.
Uros.
>
> H.J.
> ---
> gcc/
>
> PR target/68937
> * config/i386/i386.c (ix86_function_ok_for_sibcall): Count
> call via GOT slot as indirect call.
> (ix86_expand_call): Mark PIC register used for sibcall as
> call-clobbered.
> * config/i386/i386.md (*sibcall_GOT_32): New pattern.
> (*sibcall_value_GOT_32): Likewise.
>
> gcc/testsuite/
>
> PR target/68937
> * gcc.target/i386/pr68937-1.c: New test.
> * gcc.target/i386/pr68937-2.c: Likewise.
> * gcc.target/i386/pr68937-3.c: Likewise.
> * gcc.target/i386/pr68937-4.c: Likewise.
> ---
> gcc/config/i386/i386.c | 15 ++++++++---
> gcc/config/i386/i386.md | 45 +++++++++++++++++++++++++++++++
> gcc/testsuite/gcc.target/i386/pr68937-1.c | 13 +++++++++
> gcc/testsuite/gcc.target/i386/pr68937-2.c | 13 +++++++++
> gcc/testsuite/gcc.target/i386/pr68937-3.c | 13 +++++++++
> gcc/testsuite/gcc.target/i386/pr68937-4.c | 13 +++++++++
> 6 files changed, 109 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr68937-1.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr68937-2.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr68937-3.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr68937-4.c
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index cecea24..ebc9d09 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -6723,8 +6723,10 @@ ix86_function_ok_for_sibcall (tree decl, tree exp)
> /* If this call is indirect, we'll need to be able to use a
> call-clobbered register for the address of the target function.
> Make sure that all such registers are not used for passing
> - parameters. Note that DLLIMPORT functions are indirect. */
> + parameters. Note that DLLIMPORT functions and call via GOT
> + slot are indirect. */
> if (!decl
> + || (flag_pic && !flag_plt)
> || (TARGET_DLLIMPORT_DECL_ATTRIBUTES && DECL_DLLIMPORT_P (decl)))
> {
> /* Check if regparm >= 3 since arg_reg_available is set to
> @@ -27019,8 +27021,8 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1,
> rtx callarg2,
> rtx pop, bool sibcall)
> {
> - rtx vec[3];
> - rtx use = NULL, call;
> + rtx vec[4];
> + rtx use = NULL, call, clobber = NULL;
> unsigned int vec_len = 0;
>
> if (pop == const0_rtx)
> @@ -27075,6 +27077,10 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1,
> fnaddr = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, addr),
> UNSPEC_GOT);
> fnaddr = gen_rtx_CONST (Pmode, fnaddr);
> + /* Since sibcall never returns, mark PIC register as
> + call-clobbered. */
> + if (sibcall)
> + clobber = pic_offset_table_rtx;
> fnaddr = gen_rtx_PLUS (Pmode, pic_offset_table_rtx,
> fnaddr);
> }
> @@ -27151,6 +27157,9 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1,
> }
> vec[vec_len++] = call;
>
> + if (clobber)
> + vec[vec_len++] = gen_rtx_CLOBBER (VOIDmode, clobber);
> +
> if (pop)
> {
> pop = gen_rtx_PLUS (Pmode, stack_pointer_rtx, pop);
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index 49b2216..65c1534 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -11865,6 +11865,28 @@
> "* return ix86_output_call_insn (insn, operands[0]);"
> [(set_attr "type" "call")])
>
> +;; Since sibcall never returns, we can only use call-clobbered register
> +;; as GOT base.
> +(define_insn "*sibcall_GOT_32"
> + [(call (mem:QI
> + (mem:SI (plus:SI
> + (match_operand:SI 0 "register_operand" "U")
> + (const:SI
> + (unspec:SI [(match_operand:SI 1 "symbol_operand")]
> + UNSPEC_GOT)))))
> + (match_operand 2))
> + (clobber (match_dup 0))]
> + "!TARGET_MACHO && !TARGET_64BIT && SIBLING_CALL_P (insn)"
> +{
> + rtx fnaddr = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, operands[1]),
> + UNSPEC_GOT);
> + fnaddr = gen_rtx_CONST (Pmode, fnaddr);
> + fnaddr = gen_rtx_PLUS (Pmode, operands[0], fnaddr);
> + fnaddr = gen_const_mem (Pmode, fnaddr);
> + return ix86_output_call_insn (insn, fnaddr);
> +}
> + [(set_attr "type" "call")])
> +
> (define_insn "*sibcall"
> [(call (mem:QI (match_operand:W 0 "sibcall_insn_operand" "UBsBz"))
> (match_operand 1))]
> @@ -12042,6 +12064,29 @@
> "* return ix86_output_call_insn (insn, operands[1]);"
> [(set_attr "type" "callv")])
>
> +;; Since sibcall never returns, we can only use call-clobbered register
> +;; as GOT base.
> +(define_insn "*sibcall_value_GOT_32"
> + [(set (match_operand 0)
> + (call (mem:QI
> + (mem:SI (plus:SI
> + (match_operand:SI 1 "register_operand" "U")
> + (const:SI
> + (unspec:SI [(match_operand:SI 2 "symbol_operand")]
> + UNSPEC_GOT)))))
> + (match_operand 3)))
> + (clobber (match_dup 1))]
> + "!TARGET_MACHO && !TARGET_64BIT && SIBLING_CALL_P (insn)"
> +{
> + rtx fnaddr = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, operands[2]),
> + UNSPEC_GOT);
> + fnaddr = gen_rtx_CONST (Pmode, fnaddr);
> + fnaddr = gen_rtx_PLUS (Pmode, operands[1], fnaddr);
> + fnaddr = gen_const_mem (Pmode, fnaddr);
> + return ix86_output_call_insn (insn, fnaddr);
> +}
> + [(set_attr "type" "callv")])
> +
> (define_insn "*sibcall_value"
> [(set (match_operand 0)
> (call (mem:QI (match_operand:W 1 "sibcall_insn_operand" "UBsBz"))
> diff --git a/gcc/testsuite/gcc.target/i386/pr68937-1.c b/gcc/testsuite/gcc.target/i386/pr68937-1.c
> new file mode 100644
> index 0000000..166c309
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr68937-1.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile { target { *-*-linux* && ia32 } } } */
> +/* { dg-options "-O2 -fpic -fno-plt -mregparm=3" } */
> +
> +extern void bar (int);
> +
> +void
> +foo (int b)
> +{
> + bar (b);
> + bar (b);
> +}
> +
> +/* { dg-final { scan-assembler "jmp\[ \t\]*.bar@GOT\\(%e(a|c|d)x\\)" { target ia32 } } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr68937-2.c b/gcc/testsuite/gcc.target/i386/pr68937-2.c
> new file mode 100644
> index 0000000..9aa4f4d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr68937-2.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile { target { *-*-linux* && ia32 } } } */
> +/* { dg-options "-O2 -fpic -fno-plt -mregparm=3" } */
> +
> +extern void bar (int, int);
> +
> +void
> +foo (int a, int b)
> +{
> + bar (a, b);
> + bar (a, b);
> +}
> +
> +/* { dg-final { scan-assembler "jmp\[ \t\]*.bar@GOT\\(%e(a|c|d)x\\)" { target ia32 } } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr68937-3.c b/gcc/testsuite/gcc.target/i386/pr68937-3.c
> new file mode 100644
> index 0000000..6d8e40f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr68937-3.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile { target { *-*-linux* && ia32 } } } */
> +/* { dg-options "-O2 -fpic -fno-plt -mregparm=3" } */
> +
> +extern void bar (int, int, int);
> +
> +void
> +foo (int a, int b, int c)
> +{
> + bar (a, b, c);
> + bar (a, b, c);
> +}
> +
> +/* { dg-final { scan-assembler-not "jmp\[ \t\]*.bar@GOT" } } */
> diff --git a/gcc/testsuite/gcc.target/i386/pr68937-4.c b/gcc/testsuite/gcc.target/i386/pr68937-4.c
> new file mode 100644
> index 0000000..fc5f2a6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr68937-4.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile { target { *-*-linux* && ia32 } } } */
> +/* { dg-options "-O2 -fpic -fno-plt -mregparm=3" } */
> +
> +extern int bar (int, int);
> +
> +int
> +foo (int a, int b)
> +{
> + (void) bar (a, b);
> + return bar (a, b);
> +}
> +
> +/* { dg-final { scan-assembler "jmp\[ \t\]*.bar@GOT\\(%e(a|c|d)x\\)" { target ia32 } } } */
> --
> 2.5.0
>
>
next prev parent reply other threads:[~2015-12-17 10:04 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-16 23:29 H.J. Lu
2015-12-17 10:04 ` Uros Bizjak [this message]
2015-12-17 13:00 ` H.J. Lu
2015-12-17 13:42 ` Uros Bizjak
2015-12-17 15:50 ` H.J. Lu
2015-12-17 16:12 ` H.J. Lu
2015-12-17 18:09 ` H.J. Lu
2015-12-17 21:21 ` Uros Bizjak
2015-12-17 21:59 ` H.J. Lu
2015-12-18 0:55 ` H.J. Lu
2015-12-19 16:19 ` Uros Bizjak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFULd4YMDKgyWQDiE5+e1WDCJad3tiyOztVAPb4TnktVdCvqUA@mail.gmail.com \
--to=ubizjak@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hjl.tools@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).