Re: [PATCH] PR target/68937: i686: -fno-plt produces wrong code (maybe only with tailcall

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

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.

OK for master if there is no regression.

Thanks.

-- 
H.J.

From e055e1ea71353897aa7ce4b38a5186c8b64ddc7c 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.

gcc/

	PR target/68937
	* config/i386/i386.c (ix86_function_ok_for_sibcall): Count
	call 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): Remove
	GOT slot.

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/config/i386/i386.c                    |  4 ++-
 gcc/config/i386/i386.md                   | 43 +++++++++++++++++++++++++++++++
 gcc/config/i386/predicates.md             | 10 +++----
 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 +++++++
 8 files changed, 111 insertions(+), 7 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

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index cecea24..0e2bec3 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
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 49b2216..7c62586 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -11865,6 +11865,27 @@
   "* 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")
+		     (const:SI
+		       (unspec:SI [(match_operand:SI 1 "symbol_operand")]
+			UNSPEC_GOT)))))
+	 (match_operand 2))]
+  "!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 +12063,28 @@
   "* 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")
+			  (const:SI
+			    (unspec:SI [(match_operand:SI 2 "symbol_operand")]
+			     UNSPEC_GOT)))))
+	 (match_operand 3)))]
+  "!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/config/i386/predicates.md b/gcc/config/i386/predicates.md
index 8bdd5d8..006bb2c 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -597,14 +597,12 @@
 	    (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.  Don't allow GOT slot here.  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_test "CONSTANT_P (XEXP (op, 0))")))
 
 ;; Test for a valid operand for a call instruction.
 ;; Allow constant call address operands in Pmode only.
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++)
+    ;
+}
-- 
2.5.0