This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: [PATCH] PR target/66232: -fPIC -fno-plt -mx32 fails to generate indirect branch via GOT

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, 21 May 2015 11:36:19 -0700
Subject: Re: [PATCH] PR target/66232: -fPIC -fno-plt -mx32 fails to generate indirect branch via GOT
Authentication-results: sourceware.org; auth=none
References: <20150521125904 dot GA26391 at gmail dot com> <CAFULd4bB2TB1ZRYi79haFvxUMPCKgG9mPfb7C-3RcV9B_ah_xg at mail dot gmail dot com> <CAMe9rOqdUwKRMRRPpZ3ahGO7jzGuRk87efx9j=myGCQTvMLe1g at mail dot gmail dot com> <CAMe9rOo2bew_Q0MD0NQk9j+AqkrZT=ikY9zXQmQDAszNKwJYmQ at mail dot gmail dot com> <CAFULd4auNTic56yVSvVp4OP52ddCObR5-zj3dSUf64CDtr831Q at mail dot gmail dot com>

On Thu, May 21, 2015 at 11:19 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Thu, May 21, 2015 at 7:01 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>> Here is the updated patch.  It limited memory operand to
>> GOT slot only.  It used a single pattern to cover both call
>> and sibcall since only GOT slot is allowed.
>>
>> OK for master if there is no regression?
>>
>> Thanks.
>>
>>
>> --
>> H.J.
>> ---
>> X32 doesn't support indirect branch via 32-bit memory slot since
>> indirect branch will load 64-bit address from 64-bit memory slot.
>> Since x32 GOT slot is 64-bit, we should allow indirect branch via GOT
>> slot for x32.
>>
>> gcc/
>>
>> PR target/66232
>> * config/i386/constraints.md (Bg): Add a constraint for x32
>> call and sibcall memory operand.
>> * config/i386/i386.md (*call_got_x32): New pattern.
>> (*call_value_got_x32): Likewise.
>> * config/i386/predicates.md (x32_call_got_memory_operand): New
>> predicate.
>> (x32_call_insn_got_operand): Likewise.
>>
>> gcc/testsuite/
>>
>> PR target/66232
>> * gcc.target/i386/pr66232-1.c: New test.
>> * gcc.target/i386/pr66232-2.c: Likewise.
>> * gcc.target/i386/pr66232-3.c: Likewise.
>> * gcc.target/i386/pr66232-4.c: Likewise.
>> * gcc.target/i386/pr66232-5.c: Likewise.
>
> diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
> index 2271bd1..4ec9821 100644
> --- a/gcc/config/i386/constraints.md
> +++ b/gcc/config/i386/constraints.md
> @@ -146,10 +146,16 @@
>   "@internal Lower SSE register when avoiding REX prefix and all SSE
> registers otherwise.")
>
>  ;; We use the B prefix to denote any number of internal operands:
> +;;  g  Call and sibcall memory operand, valid for TARGET_X32
>  ;;  s  Sibcall memory operand, not valid for TARGET_X32
>  ;;  w  Call memory operand, not valid for TARGET_X32
>  ;;  z  Constant call address operand.
>
> +(define_constraint "Bg"
> +  "@internal Call/sibcall GOT memory operand for x32."
> +  (and (match_test "TARGET_X32")
> +       (match_operand 0 "x32_call_got_memory_operand")))
>
> No need for TARGET_X32, when all insn patterns are protected by it.
> And the predicate is not x32 specific. And not call specific, so
> "@internal GOT memory operand". Please use "GOT_memory_operand"
> predicate, see below.
>
>  (define_constraint "Bs"
>    "@internal Sibcall memory operand."
>    (and (not (match_test "TARGET_X32"))
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index aefca43..158a3ed 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -11659,6 +11659,15 @@
>    "* return ix86_output_call_insn (insn, operands[0]);"
>    [(set_attr "type" "call")])
>
> +;; This covers both call and sibcall since only GOT slot is allowed.
> +(define_insn "*call_got_x32"
> +  [(call (mem:QI (zero_extend:DI
> +           (match_operand:SI 0 "x32_call_insn_got_operand" "Bg")))
> +     (match_operand 1))]
> +  "TARGET_X32"
> +  "* return ix86_output_call_insn (insn, operands[0]);"
> +  [(set_attr "type" "call")])
>
> "GOT_memory_operand" in the above pattern and all other call patterns.
>
>  (define_insn "*sibcall"
>    [(call (mem:QI (match_operand:W 0 "sibcall_insn_operand" "UBsBz"))
>       (match_operand 1))]
> @@ -11825,6 +11834,17 @@
>    "* return ix86_output_call_insn (insn, operands[1]);"
>    [(set_attr "type" "callv")])
>
> +;; This covers both call and sibcall since only GOT slot is allowed.
> +(define_insn "*call_value_got_x32"
> +  [(set (match_operand 0)
> +    (call (mem:QI
> +        (zero_extend:DI
> +          (match_operand:SI 1 "x32_call_insn_got_operand" "Bg")))
> +          (match_operand 2)))]
> +  "TARGET_X32"
> +  "* return ix86_output_call_insn (insn, operands[1]);"
> +  [(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 26dd3e1..166893e 100644
> --- a/gcc/config/i386/predicates.md
> +++ b/gcc/config/i386/predicates.md
> @@ -606,6 +606,20 @@
>         (and (not (match_test "TARGET_X32"))
>          (match_operand 0 "sibcall_memory_operand"))))
>
> +;; Return true if OP is a GOT memory operand that can be used in x32 calls
> +;; and sibcalls.
> +(define_predicate "x32_call_got_memory_operand"
> +  (and (match_operand 0 "memory_operand")
> +       (match_test "CONSTANT_P (XEXP (op, 0))")
> +       (match_test "GET_CODE (XEXP (XEXP (op, 0), 0)) == UNSPEC")
> +       (match_test "XINT (XEXP (XEXP (op, 0), 0), 1) == UNSPEC_GOTPCREL")))
>
> There is nothing x32 specific, it is just a GOT memory operand, so
> let's name it "GOT_memory_operand".
>
> +;; Test for a valid operand for an x32 call/sibcall instruction via
> +;; memory.  Only the 64-bit GOT slot is allowed.
> +(define_special_predicate "x32_call_insn_got_operand"
> +  (and (match_test "TARGET_X32")
> +       (match_operand 0 "x32_call_got_memory_operand")))
>
> The above predicate is not needed, all insns are protected by TARGET_X32.
>
>  ;; Match exactly zero.
>  (define_predicate "const0_operand"

I am testing this now.  I will check it in if there is no regression.

Thanks.

-- 
H.J.
---
X32 doesn't support indirect branch via 32-bit memory slot since
indirect branch will load 64-bit address from 64-bit memory slot.
Since x32 GOT slot is 64-bit, we should allow indirect branch via GOT
slot for x32.

gcc/

PR target/66232
* config/i386/constraints.md (Bg): New.  A constraint for GOT
memory operand.
* config/i386/i386.md (*call_got_x32): New pattern.
(*call_value_got_x32): Likewise.
* config/i386/predicates.md (GOT_memory_operand): New predicate.

gcc/testsuite/

PR target/66232
* gcc.target/i386/pr66232-1.c: New test.
* gcc.target/i386/pr66232-2.c: Likewise.
* gcc.target/i386/pr66232-3.c: Likewise.
* gcc.target/i386/pr66232-4.c: Likewise.
* gcc.target/i386/pr66232-5.c: Likewise.

From da4bc478e87481b02152d23ce154ba28a0e52300 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 21 May 2015 05:50:14 -0700
Subject: [PATCH] Allow indirect branch via GOT slot for x32

X32 doesn't support indirect branch via 32-bit memory slot since
indirect branch will load 64-bit address from 64-bit memory slot.
Since x32 GOT slot is 64-bit, we should allow indirect branch via GOT
slot for x32.

gcc/

	PR target/66232
	* config/i386/constraints.md (Bg): New.  A constraint for GOT
	memory operand.
	* config/i386/i386.md (*call_got_x32): New pattern.
	(*call_value_got_x32): Likewise.
	* config/i386/predicates.md (GOT_memory_operand): New predicate.

gcc/testsuite/

	PR target/66232
	* gcc.target/i386/pr66232-1.c: New test.
	* gcc.target/i386/pr66232-2.c: Likewise.
	* gcc.target/i386/pr66232-3.c: Likewise.
	* gcc.target/i386/pr66232-4.c: Likewise.
	* gcc.target/i386/pr66232-5.c: Likewise.
---
 gcc/config/i386/constraints.md            |  5 +++++
 gcc/config/i386/i386.md                   | 20 ++++++++++++++++++++
 gcc/config/i386/predicates.md             |  7 +++++++
 gcc/testsuite/gcc.target/i386/pr66232-1.c | 13 +++++++++++++
 gcc/testsuite/gcc.target/i386/pr66232-2.c | 14 ++++++++++++++
 gcc/testsuite/gcc.target/i386/pr66232-3.c | 13 +++++++++++++
 gcc/testsuite/gcc.target/i386/pr66232-4.c | 13 +++++++++++++
 gcc/testsuite/gcc.target/i386/pr66232-5.c | 16 ++++++++++++++++
 8 files changed, 101 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr66232-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr66232-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr66232-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr66232-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr66232-5.c

diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
index 2271bd1..c718bc1 100644
--- a/gcc/config/i386/constraints.md
+++ b/gcc/config/i386/constraints.md
@@ -146,10 +146,15 @@
  "@internal Lower SSE register when avoiding REX prefix and all SSE registers otherwise.")
 
 ;; We use the B prefix to denote any number of internal operands:
+;;  g  GOT memory operand.
 ;;  s  Sibcall memory operand, not valid for TARGET_X32
 ;;  w  Call memory operand, not valid for TARGET_X32
 ;;  z  Constant call address operand.
 
+(define_constraint "Bg"
+  "@internal GOT memory operand."
+  (match_operand 0 "GOT_memory_operand"))
+
 (define_constraint "Bs"
   "@internal Sibcall memory operand."
   (and (not (match_test "TARGET_X32"))
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index aefca43..3819dfd 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -11659,6 +11659,15 @@
   "* return ix86_output_call_insn (insn, operands[0]);"
   [(set_attr "type" "call")])
 
+;; This covers both call and sibcall since only GOT slot is allowed.
+(define_insn "*call_got_x32"
+  [(call (mem:QI (zero_extend:DI
+		   (match_operand:SI 0 "GOT_memory_operand" "Bg")))
+	 (match_operand 1))]
+  "TARGET_X32"
+  "* return ix86_output_call_insn (insn, operands[0]);"
+  [(set_attr "type" "call")])
+
 (define_insn "*sibcall"
   [(call (mem:QI (match_operand:W 0 "sibcall_insn_operand" "UBsBz"))
 	 (match_operand 1))]
@@ -11825,6 +11834,17 @@
   "* return ix86_output_call_insn (insn, operands[1]);"
   [(set_attr "type" "callv")])
 
+;; This covers both call and sibcall since only GOT slot is allowed.
+(define_insn "*call_value_got_x32"
+  [(set (match_operand 0)
+	(call (mem:QI
+		(zero_extend:DI
+		  (match_operand:SI 1 "GOT_memory_operand" "Bg")))
+	      (match_operand 2)))]
+  "TARGET_X32"
+  "* return ix86_output_call_insn (insn, operands[1]);"
+  [(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 26dd3e1..009aa24 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -606,6 +606,13 @@
        (and (not (match_test "TARGET_X32"))
 	    (match_operand 0 "sibcall_memory_operand"))))
 
+;; Return true if OP is a GOT memory operand.
+(define_predicate "GOT_memory_operand"
+  (and (match_operand 0 "memory_operand")
+       (match_test "CONSTANT_P (XEXP (op, 0))")
+       (match_test "GET_CODE (XEXP (XEXP (op, 0), 0)) == UNSPEC")
+       (match_test "XINT (XEXP (XEXP (op, 0), 0), 1) == UNSPEC_GOTPCREL")))
+
 ;; 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/pr66232-1.c b/gcc/testsuite/gcc.target/i386/pr66232-1.c
new file mode 100644
index 0000000..ba4a5ef
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr66232-1.c
@@ -0,0 +1,13 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpic -fno-plt" } */
+
+extern void bar (void);
+
+void
+foo (void)
+{
+  bar ();
+}
+
+/* { dg-final { scan-assembler "jmp\[ \t\]*.bar@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "call\[ \t\]*.bar@GOT\\(" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr66232-2.c b/gcc/testsuite/gcc.target/i386/pr66232-2.c
new file mode 100644
index 0000000..f05d7c5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr66232-2.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpic -fno-plt" } */
+
+extern void bar (void);
+
+int
+foo (void)
+{
+  bar ();
+  return 0;
+}
+
+/* { dg-final { scan-assembler "call\[ \t\]*.bar@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "call\[ \t\]*.bar@GOT\\(" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr66232-3.c b/gcc/testsuite/gcc.target/i386/pr66232-3.c
new file mode 100644
index 0000000..ee3176c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr66232-3.c
@@ -0,0 +1,13 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpic -fno-plt" } */
+
+extern int bar (void);
+
+int
+foo (void)
+{
+  return bar ();
+}
+
+/* { dg-final { scan-assembler "jmp\[ \t\]*.bar@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "call\[ \t\]*.bar@GOT\\(" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr66232-4.c b/gcc/testsuite/gcc.target/i386/pr66232-4.c
new file mode 100644
index 0000000..5fa0ded
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr66232-4.c
@@ -0,0 +1,13 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpic -fno-plt" } */
+
+extern int bar (void);
+
+int
+foo (void)
+{
+  return bar () + 1;
+}
+
+/* { dg-final { scan-assembler "call\[ \t\]*.bar@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "call\[ \t\]*.bar@GOT\\(" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr66232-5.c b/gcc/testsuite/gcc.target/i386/pr66232-5.c
new file mode 100644
index 0000000..c016717
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr66232-5.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-require-effective-target maybe_x32 } */
+/* { dg-options "-O2 -fpic -mx32" } */
+
+extern void (*bar) (void);
+void
+foo (int n)
+{
+  int i;
+  for (i = 0; i < n; i++)
+    {
+      if (!bar)
+	continue;
+      (*bar) ();
+    }
+}
-- 
1.9.3

Follow-Ups:
- Re: [PATCH] PR target/66232: -fPIC -fno-plt -mx32 fails to generate indirect branch via GOT
  - From: Uros Bizjak

References:
- [PATCH] PR target/66232: -fPIC -fno-plt -mx32 fails to generate indirect branch via GOT
  - From: H.J. Lu
- Re: [PATCH] PR target/66232: -fPIC -fno-plt -mx32 fails to generate indirect branch via GOT
  - From: Uros Bizjak
- Re: [PATCH] PR target/66232: -fPIC -fno-plt -mx32 fails to generate indirect branch via GOT
  - From: H.J. Lu
- Re: [PATCH] PR target/66232: -fPIC -fno-plt -mx32 fails to generate indirect branch via GOT
  - From: H.J. Lu
- Re: [PATCH] PR target/66232: -fPIC -fno-plt -mx32 fails to generate indirect branch via GOT
  - From: Uros Bizjak

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]