This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] PR target/68937: i686: -fno-plt produces wrong code (maybe only with tailcall
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Uros Bizjak <ubizjak at gmail dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 17 Dec 2015 16:55:35 -0800
- Subject: Re: [PATCH] PR target/68937: i686: -fno-plt produces wrong code (maybe only with tailcall
- Authentication-results: sourceware.org; auth=none
- References: <20151216232951 dot GA17976 at intel dot com> <CAFULd4YMDKgyWQDiE5+e1WDCJad3tiyOztVAPb4TnktVdCvqUA at mail dot gmail dot com> <CAMe9rOqK2XWKzNJERP2YZTKYMSD7r5pbb3bqv4e146Gjro867Q at mail dot gmail dot com> <CAFULd4b2O=09RFYKp0C80yFx=W_yJXjJxU+a=WZGuzs3r3y+RQ at mail dot gmail dot com> <CAMe9rOocFnSi5Th1Gtko8Nq7zFUsm-+M0Leus5kt5RD=vO26og at mail dot gmail dot com> <CAMe9rOoLARSd6WPy_dr6JTJ+NPdrEW6eLhXZ77-UOuLSL1EcYw at mail dot gmail dot com> <CAMe9rOoAAC+aUV+zGwCfwmHiTdEv72ZceQ-BET7_Fc=6a0rF2g at mail dot gmail dot com> <CAFULd4YySPUriup-g-ozC-c+8Kj_eXSvAE4cqS1dLnxaRoPMZA at mail dot gmail dot com> <CAMe9rOpL_UhrSvePGUrX6bzUTvJ=M+5tfoJ+ZLM61hKJtK26bQ at mail dot gmail dot com>
On Thu, Dec 17, 2015 at 1:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Dec 17, 2015 at 1:21 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Thu, Dec 17, 2015 at 7:09 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Thu, Dec 17, 2015 at 8:11 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Thu, Dec 17, 2015 at 7:50 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Thu, Dec 17, 2015 at 5:42 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>>>> On Thu, Dec 17, 2015 at 2:00 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>> On Thu, Dec 17, 2015 at 2:04 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>>>>>> 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.
>>>>>>>>
>>>>>>>
>>>>>>> Here is the updated patch. There is a predicate already,
>>>>>>> sibcall_memory_operand. It allows any registers to
>>>>>>> be as GOT base, which is the root of our problem.
>>>>>>> This patch removes GOT slot from it and handles
>>>>>>> sibcall over GOT slot with *sibcall_GOT_32 and
>>>>>>> *sibcall_value_GOT_32 patterns. Since I need to
>>>>>>> expose constraints on GOT base register to RA,
>>>>>>> I have to use 2 operands, GOT base and function
>>>>>>> symbol, to describe sibcall over 32-bit GOT slot.
>>>>>>
>>>>>> Please use
>>>>>>
>>>>>> (mem:SI (plus:SI
>>>>>> (match_operand:SI 0 "register_no_elim_operand" "U")
>>>>>> (match_operand:SI 1 "GOT32_symbol_operand")))
>>>>>> ...
>>>>>>
>>>>>> to avoid manual rebuild of the operand.
>>>>>>
>>>>>
>>>>> Is this OK?
>>>>>
>>>>
>>>> An updated patch to allow sibcall_memory_operand for RTL
>>>> expansion. OK for trunk if there is no regression?
>>>>
>>>
>>> There is no regressions on x86-64 with -m32. OK for trunk?
>>
>> OK for mainline, with a following change:
>>
>> @@ -597,11 +597,17 @@
>> (match_operand 0 "memory_operand"))))
>>
>> ;; Return true if OP is a memory operands that can be used in sibcalls.
>> +;; Since sibcall never returns, we can only use call-clobbered register
>> +;; as GOT base. Allow GOT slot here only with pseudo register as GOT
>> +;; base. Properly handle sibcall over GOT slot with *sibcall_GOT_32
>> +;; and *sibcall_value_GOT_32 patterns.
>> (define_predicate "sibcall_memory_operand"
>> (and (match_operand 0 "memory_operand")
>> (match_test "CONSTANT_P (XEXP (op, 0))
>> || (GET_CODE (XEXP (op, 0)) == PLUS
>> && REG_P (XEXP (XEXP (op, 0), 0))
>> + && (REGNO (XEXP (XEXP (op, 0), 0))
>> + >= FIRST_PSEUDO_REGISTER)
>> && GET_CODE (XEXP (XEXP (op, 0), 1)) == CONST
>> && GET_CODE (XEXP (XEXP (XEXP (op, 0), 1), 0)) == UNSPEC
>> && XINT (XEXP (XEXP (XEXP (op, 0), 1), 0), 1) == UNSPEC_GOT)")))
>>
>> You can use (!HARD_REGISTER_NUM_P (...) || call_used_regs[...]) here.
>> Call-used hard regs are still allowed here.
>>
>> Can you please also rewrite this horrible match_test as a block of C
>> code using GOT32_symbol_operand predicate?
>>
>
> I am retesting the patch with
>
> ;; Return true if OP is a memory operands that can be used in sibcalls.
> ;; Since sibcall never returns, we can only use call-clobbered register
> ;; as GOT base. Allow GOT slot here only with pseudo register as GOT
> ;; base. Properly handle sibcall over GOT slot with *sibcall_GOT_32
> ;; and *sibcall_value_GOT_32 patterns.
> (define_predicate "sibcall_memory_operand"
> (match_operand 0 "memory_operand")
> {
> op = XEXP (op, 0);
> if (CONSTANT_P (op))
> return true;
> if (GET_CODE (op) == PLUS && REG_P (XEXP (op, 0)))
> {
> int regno = REGNO (XEXP (op, 0));
> if (!HARD_REGISTER_NUM_P (regno) || call_used_regs[regno])
> {
> op = XEXP (op, 1);
> if (GOT32_symbol_operand (op, VOIDmode))
> return true;
> }
> }
> return false;
> })
>
>
> I will check it in if there is no regression.
>
There is no regression. But I missed sibcall to local function
with -O2 -fPIC -m32 -fno-plt -mregparm=3:
extern void bar (int, int, int) __attribute__((visibility("hidden")));
void
foo (int a, int b, int c)
{
bar (a, b, c);
bar (a, b, c);
}
It doesn't need GOT. This patch fixes it.
iff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 0e2bec3..691915f9 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -6657,6 +6657,7 @@ ix86_function_ok_for_sibcall (tree decl, tree exp)
{
tree type, decl_or_type;
rtx a, b;
+ bool bind_global = decl && !targetm.binds_local_p (decl);
/* If we are generating position-independent code, we cannot sibcall
optimize direct calls to global functions, as the PLT requires
@@ -6665,7 +6666,7 @@ ix86_function_ok_for_sibcall (tree decl, tree exp)
&& !TARGET_64BIT
&& flag_pic
&& flag_plt
- && decl && !targetm.binds_local_p (decl))
+ && bind_global)
return false;
/* If we need to align the outgoing stack, then sibcalling would
@@ -6726,7 +6727,7 @@ ix86_function_ok_for_sibcall (tree decl, tree exp)
parameters. Note that DLLIMPORT functions and call via GOT
slot are indirect. */
if (!decl
- || (flag_pic && !flag_plt)
+ || (bind_global && flag_pic && !flag_plt)
|| (TARGET_DLLIMPORT_DECL_ATTRIBUTES && DECL_DLLIMPORT_P (decl)))
{
/* Check if regparm >= 3 since arg_reg_available is set to
Here is the complete patch with a testcase. OK for trunk?
Thanks.
--
H.J.
From e4a7e066a8cdbb638fc5e1312c01f1043a2679fc Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 16 Dec 2015 12:34:57 -0800
Subject: [PATCH] Use call-clobbered register for sibcall via GOT
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. sibcall_memory_operand is changed to allow 32-bit
GOT slot only with pseudo register as GOT base for RTL expansion. 2
new patterns, *sibcall_GOT_32 and *sibcall_value_GOT_32, are added to
expose GOT base register to register allocator so that call-clobbered
register will be used for GOT base.
gcc/
PR target/68937
* config/i386/i386.c (ix86_function_ok_for_sibcall): Count
call to global function via GOT slot as indirect call.
* config/i386/i386.md (*sibcall_GOT_32): New pattern.
(*sibcall_value_GOT_32): Likewise.
* config/i386/predicates.md (sibcall_memory_operand): Rewrite.
Allow 32-bit GOT slot only with pseudo register as GOT base.
(GOT32_symbol_operand): New predicate.
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.target/i386/pr68937-5.c: Likewise.
* gcc.target/i386/pr68937-6.c: Likewise.
---
gcc/config/i386/i386.c | 7 +++++--
gcc/config/i386/i386.md | 33 ++++++++++++++++++++++++++++++
gcc/config/i386/predicates.md | 34 ++++++++++++++++++++++++-------
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 ++++++++++++
gcc/testsuite/gcc.target/i386/pr68937-5.c | 9 ++++++++
gcc/testsuite/gcc.target/i386/pr68937-6.c | 16 +++++++++++++++
9 files changed, 142 insertions(+), 9 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
create mode 100644 gcc/testsuite/gcc.target/i386/pr68937-5.c
create mode 100644 gcc/testsuite/gcc.target/i386/pr68937-6.c
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index cecea24..f5d23d9 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -6657,6 +6657,7 @@ ix86_function_ok_for_sibcall (tree decl, tree exp)
{
tree type, decl_or_type;
rtx a, b;
+ bool bind_global = decl && !targetm.binds_local_p (decl);
/* If we are generating position-independent code, we cannot sibcall
optimize direct calls to global functions, as the PLT requires
@@ -6665,7 +6666,7 @@ ix86_function_ok_for_sibcall (tree decl, tree exp)
&& !TARGET_64BIT
&& flag_pic
&& flag_plt
- && decl && !targetm.binds_local_p (decl))
+ && bind_global)
return false;
/* If we need to align the outgoing stack, then sibcalling would
@@ -6723,8 +6724,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 to global
+ function via GOT slot are indirect. */
if (!decl
+ || (bind_global && flag_pic && !flag_plt)
|| (TARGET_DLLIMPORT_DECL_ATTRIBUTES && DECL_DLLIMPORT_P (decl)))
{
/* Check if regparm >= 3 since arg_reg_available is set to
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 49b2216..6ab8eaa 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -11865,6 +11865,22 @@
"* 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_no_elim_operand" "U")
+ (match_operand:SI 1 "GOT32_symbol_operand"))))
+ (match_operand 2))]
+ "!TARGET_MACHO && !TARGET_64BIT && SIBLING_CALL_P (insn)"
+{
+ rtx fnaddr = gen_rtx_PLUS (Pmode, operands[0], operands[1]);
+ 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 +12058,23 @@
"* 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_no_elim_operand" "U")
+ (match_operand:SI 2 "GOT32_symbol_operand"))))
+ (match_operand 3)))]
+ "!TARGET_MACHO && !TARGET_64BIT && SIBLING_CALL_P (insn)"
+{
+ rtx fnaddr = gen_rtx_PLUS (Pmode, operands[1], operands[2]);
+ 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/config/i386/predicates.md b/gcc/config/i386/predicates.md
index 8bdd5d8..96d946c 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -597,14 +597,28 @@
(match_operand 0 "memory_operand"))))
;; Return true if OP is a memory operands that can be used in sibcalls.
+;; Since sibcall never returns, we can only use call-clobbered register
+;; as GOT base. Allow GOT slot here only with pseudo register as GOT
+;; base. Properly handle sibcall over GOT slot with *sibcall_GOT_32
+;; and *sibcall_value_GOT_32 patterns.
(define_predicate "sibcall_memory_operand"
- (and (match_operand 0 "memory_operand")
- (match_test "CONSTANT_P (XEXP (op, 0))
- || (GET_CODE (XEXP (op, 0)) == PLUS
- && REG_P (XEXP (XEXP (op, 0), 0))
- && GET_CODE (XEXP (XEXP (op, 0), 1)) == CONST
- && GET_CODE (XEXP (XEXP (XEXP (op, 0), 1), 0)) == UNSPEC
- && XINT (XEXP (XEXP (XEXP (op, 0), 1), 0), 1) == UNSPEC_GOT)")))
+ (match_operand 0 "memory_operand")
+{
+ op = XEXP (op, 0);
+ if (CONSTANT_P (op))
+ return true;
+ if (GET_CODE (op) == PLUS && REG_P (XEXP (op, 0)))
+ {
+ int regno = REGNO (XEXP (op, 0));
+ if (!HARD_REGISTER_NUM_P (regno) || call_used_regs[regno])
+ {
+ op = XEXP (op, 1);
+ if (GOT32_symbol_operand (op, VOIDmode))
+ return true;
+ }
+ }
+ return false;
+})
;; Test for a valid operand for a call instruction.
;; Allow constant call address operands in Pmode only.
@@ -633,6 +647,12 @@
&& XINT (XEXP (op, 0), 1) == UNSPEC_GOTPCREL);
})
+;; Return true if OP is a 32-bit GOT symbol operand.
+(define_predicate "GOT32_symbol_operand"
+ (match_test "GET_CODE (op) == CONST
+ && GET_CODE (XEXP (op, 0)) == UNSPEC
+ && XINT (XEXP (op, 0), 1) == UNSPEC_GOT"))
+
;; Match exactly zero.
(define_predicate "const0_operand"
(match_code "const_int,const_wide_int,const_double,const_vector")
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..897856b
--- /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\\)" } } */
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..257f4e2
--- /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\\)" } } */
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..9c19956
--- /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\\)" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr68937-5.c b/gcc/testsuite/gcc.target/i386/pr68937-5.c
new file mode 100644
index 0000000..f7e3ec5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr68937-5.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { *-*-linux* } } } */
+/* { dg-options "-O2 -fpic -fno-plt -funroll-loops" } */
+
+extern void *f();
+void dmi_scan_machine(void) {
+ char *p = f(), *q;
+ for (q = p; q < p + 10; q++)
+ ;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr68937-6.c b/gcc/testsuite/gcc.target/i386/pr68937-6.c
new file mode 100644
index 0000000..406ce28
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr68937-6.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target { *-*-linux* && ia32 } } } */
+/* { dg-options "-O2 -fpic -fno-plt -mregparm=3" } */
+
+extern void foo (int, int, int);
+extern void bar (int, int, int) __attribute__((visibility("hidden")));
+
+void
+foo (int a, int b, int c)
+{
+ foo (a, b, c);
+ bar (a, b, c);
+ foo (a, b, c);
+ bar (a, b, c);
+}
+
+/* { dg-final { scan-assembler "jmp\[ \t\]bar" } } */
--
2.5.0