On Mon, Jan 20, 2020 at 5:24 AM H.J. Lu <hjl.to...@gmail.com> wrote:
>
> On Sun, Jan 19, 2020 at 11:53 PM Uros Bizjak <ubiz...@gmail.com> wrote:
> >
> > On Sun, Jan 19, 2020 at 10:00 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> > >
> > > On Sun, Jan 19, 2020 at 12:16 PM Uros Bizjak <ubiz...@gmail.com> wrote:
> > > >
> > > > On Sun, Jan 19, 2020 at 9:07 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> > > > >
> > > > > On Sun, Jan 19, 2020 at 12:01 PM Uros Bizjak <ubiz...@gmail.com> 
> > > > > wrote:
> > > > > >
> > > > > > On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> > > > > > >
> > > > > > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak <ubiz...@gmail.com> 
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak <ubiz...@gmail.com> 
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu <hjl.to...@gmail.com> 
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to 
> > > > > > > > > > replace DI with
> > > > > > > > > > P in GNU2 TLS patterns.  Since thread pointer is in 
> > > > > > > > > > ptr_mode, PLUS in
> > > > > > > > > > GNU2 TLS address computation must be done in ptr_mode to 
> > > > > > > > > > support
> > > > > > > > > > -maddress-mode=long.  Also drop the "q" suffix from lea to 
> > > > > > > > > > support
> > > > > > > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), 
> > > > > > > > > > %rax".
> > > > > > > > >
> > > > > > > > > Please use "lea%z0" instead.
> > > > > > > > >
> > > > > > > > > > Tested on Linux/x86-64.  OK for master?
> > > > > > > > > >
> > > > > > > > > > Thanks.
> > > > > > > > > >
> > > > > > > > > > H.J.
> > > > > > > > > > ---
> > > > > > > > > > gcc/
> > > > > > > > > >
> > > > > > > > > >         PR target/93319
> > > > > > > > > >         * config/i386/i386.c (legitimize_tls_address): Pass 
> > > > > > > > > > Pmode to
> > > > > > > > > >         gen_tls_dynamic_gnu2_64.  Compute GNU2 TLS address 
> > > > > > > > > > in ptr_mode.
> > > > > > > > > >         * config/i386/i386.md (tls_dynamic_gnu2_64): 
> > > > > > > > > > Renamed to ...
> > > > > > > > > >         (@tls_dynamic_gnu2_64_<mode>): This.  Replace DI 
> > > > > > > > > > with P.
> > > > > > > > > >         (*tls_dynamic_gnu2_lea_64): Renamed to ...
> > > > > > > > > >         (*tls_dynamic_gnu2_lea_64_<mode>): This.  Replace 
> > > > > > > > > > DI with P.
> > > > > > > > > >         Remove the {q} suffix from lea.
> > > > > > > > > >         (*tls_dynamic_gnu2_call_64): Renamed to ...
> > > > > > > > > >         (*tls_dynamic_gnu2_call_64_<mode>): This.  Replace 
> > > > > > > > > > DI with P.
> > > > > > > > > >         (*tls_dynamic_gnu2_combine_64): Renamed to ...
> > > > > > > > > >         (*tls_dynamic_gnu2_combine_64_<mode>): This.  
> > > > > > > > > > Replace DI with P.
> > > > > > > > > >         Pass Pmode to gen_tls_dynamic_gnu2_64.
> > > > > > > > > >
> > > > > > > > > > gcc/testsuite/
> > > > > > > > > >
> > > > > > > > > >         PR target/93319
> > > > > > > > > >         * gcc.target/i386/pr93319-1a.c: New test.
> > > > > > > > > >         * gcc.target/i386/pr93319-1b.c: Likewise.
> > > > > > > > > >         * gcc.target/i386/pr93319-1c.c: Likewise.
> > > > > > > > > >         * gcc.target/i386/pr93319-1d.c: Likewise.
> > > > > > > > > > ---
> > > > > > > > > >  gcc/config/i386/i386.c                     | 31 
> > > > > > > > > > +++++++++++--
> > > > > > > > > >  gcc/config/i386/i386.md                    | 54 
> > > > > > > > > > +++++++++++-----------
> > > > > > > > > >  gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++++++++++
> > > > > > > > > >  gcc/testsuite/gcc.target/i386/pr93319-1b.c |  7 +++
> > > > > > > > > >  gcc/testsuite/gcc.target/i386/pr93319-1c.c |  7 +++
> > > > > > > > > >  gcc/testsuite/gcc.target/i386/pr93319-1d.c |  7 +++
> > > > > > > > > >  6 files changed, 99 insertions(+), 31 deletions(-)
> > > > > > > > > >  create mode 100644 
> > > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c
> > > > > > > > > >  create mode 100644 
> > > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c
> > > > > > > > > >  create mode 100644 
> > > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c
> > > > > > > > > >  create mode 100644 
> > > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c
> > > > > > > > > >
> > > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > > > > > > > > > index 2c087a4a3e0..8c437dbe1f3 100644
> > > > > > > > > > --- a/gcc/config/i386/i386.c
> > > > > > > > > > +++ b/gcc/config/i386/i386.c
> > > > > > > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, 
> > > > > > > > > > enum tls_model model, bool for_mov)
> > > > > > > > > >        if (TARGET_GNU2_TLS)
> > > > > > > > > >         {
> > > > > > > > > >           if (TARGET_64BIT)
> > > > > > > > > > -           emit_insn (gen_tls_dynamic_gnu2_64 (dest, x));
> > > > > > > > > > +           emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, 
> > > > > > > > > > dest, x));
> > > > > > > > > >           else
> > > > > > > > > >             emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, 
> > > > > > > > > > pic));
> > > > > > > > > >
> > > > > > > > > >           tp = get_thread_pointer (Pmode, true);
> > > > > > > > > > -         dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, 
> > > > > > > > > > dest));
> > > > > > > > > > +
> > > > > > > > > > +         /* NB: Since thread pointer is in ptr_mode, make 
> > > > > > > > > > sure that
> > > > > > > > > > +            PLUS is done in ptr_mode.  */
> > > > > > > >
> > > > > > > > Actually, thread_pointer is in Pmode, see the line just above 
> > > > > > > > your
> > > > > > > > change. Also, dest is in Pmode, so why do we need all this 
> > > > > > > > subreg
> > > > > > > > dance?
> > > > > > >
> > > > > > > dest set from  gen_tls_dynamic_gnu2_64 is in ptr_mode by
> > > > > > >
> > > > > > > call *foo@TLSCALL(%rax)
> > > > > >
> > > > > > It looks to me that we have Pmode/ptr_mode mixup in
> > > > > > legitimize_tls_address & friends for TARGET_GNU2_TLS. I think that 
> > > > > > we
> > > > > > need to perform all calculations in ptr_mode, since 
> > > > > > get_thread_pointer
> > > > > > in effect always returns ptr_mode (SImode) value, and also the above
> > > > > > call always returns ptr_mode (SImode) value. In case of
> > > > > > -maddress-mode=long, the value would be zero-extended to Pmode.
> > > > > >
> > > > >
> > > > > The problem is with tls_dynamic_gnu2_64 which generates
> > > > >
> > > > > call *foo@TLSCALL(%rax)
> > > > >
> > > > > It always returns a 32-bit index.  I can change get_thread_pointer if
> > > > > needed.
> > > > >
> > > > > Here is the updated patch with "lea%z0" and updated comments.
> > > >
> > > > There is && 0 in the first if, which looks wrong.
> > > >
> > > > +      /* NB: Since DEST set by tls_dynamic_gnu2_64 is in ptr_mode,
> > > > +         make sure that PLUS is done in ptr_mode.  */
> > > > +      if (0 && Pmode != ptr_mode)
> > > > +        {
> > > >
> > >
> > > Oops.  Here is the fixed patch.
> >
> > OK. Let's go with this version, but please investigate if we need to
> > calculate TLS address in ptr_mode instead of Pmode. Due to quite some
> > zero-extension from ptr_mode to Pmode hacks in this area, it looks to
> > me that the whole calculation should be performed in ptr_mode (SImode
> > in case of x32), and the result zero-extended to Pmode in case when
> > Pmode = DImode.
> >
>
> I checked it in.  I will investigate if we can use ptr_mode for TLS.

Here is a patch to perform GNU2 TLS address computation in ptr_mode
and zero-extend result to Pmode.

-- 
H.J.
From 19e15ed1e2bf6bd354e0a5a00d07782b72642b82 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Mon, 20 Jan 2020 13:30:04 -0800
Subject: [PATCH] Do GNU2 TLS address computation in ptr_mode

Since GNU2 TLS address from glibc run-time is in ptr_mode, we can do
GNU2 TLS address computation in ptr_mode and zero-extend result to
Pmode.

	* config/i386/i386.c (ix86_tls_module_base): Replace Pmode
	with ptr_mode.
	(legitimize_tls_address): Do GNU2 TLS address computation in
	ptr_mode and zero-extend result to Pmode.
	*  config/i386/i386.md (@tls_dynamic_gnu2_64_<mode>): Replace
	:P with :PTR and Pmode with ptr_mode.
	(*tls_dynamic_gnu2_lea_64_<mode>): Likewise.
	(*tls_dynamic_gnu2_call_64_<mode>): Likewise.
	(*tls_dynamic_gnu2_combine_64_<mode>): Likewise.
---
 gcc/config/i386/i386.c  | 46 +++++++++++++++++----------------------
 gcc/config/i386/i386.md | 48 ++++++++++++++++++++---------------------
 2 files changed, 44 insertions(+), 50 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 0b8a4b9ee4f..1ffefcb74e8 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10717,7 +10717,7 @@ ix86_tls_module_base (void)
   if (!ix86_tls_module_base_symbol)
     {
       ix86_tls_module_base_symbol
-	= gen_rtx_SYMBOL_REF (Pmode, "_TLS_MODULE_BASE_");
+	= gen_rtx_SYMBOL_REF (ptr_mode, "_TLS_MODULE_BASE_");
 
       SYMBOL_REF_FLAGS (ix86_tls_module_base_symbol)
 	|= TLS_MODEL_GLOBAL_DYNAMIC << SYMBOL_FLAG_TLS_SHIFT;
@@ -10748,7 +10748,7 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov)
   switch (model)
     {
     case TLS_MODEL_GLOBAL_DYNAMIC:
-      dest = gen_reg_rtx (Pmode);
+      dest = gen_reg_rtx (TARGET_GNU2_TLS ? ptr_mode : Pmode);
 
       if (!TARGET_64BIT)
 	{
@@ -10764,23 +10764,14 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov)
       if (TARGET_GNU2_TLS)
 	{
 	  if (TARGET_64BIT)
-	    emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x));
+	    emit_insn (gen_tls_dynamic_gnu2_64 (ptr_mode, dest, x));
 	  else
 	    emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic));
 
-	  tp = get_thread_pointer (Pmode, true);
-
-	  /* NB: Since DEST set by tls_dynamic_gnu2_64 is in ptr_mode,
-	     make sure that PLUS is done in ptr_mode.  */
-	  if (Pmode != ptr_mode)
-	    {
-	      tp = lowpart_subreg (ptr_mode, tp, Pmode);
-	      dest = lowpart_subreg (ptr_mode, dest, Pmode);
-	      dest = gen_rtx_PLUS (ptr_mode, tp, dest);
-	      dest = gen_rtx_ZERO_EXTEND (Pmode, dest);
-	    }
-	  else
-	    dest = gen_rtx_PLUS (Pmode, tp, dest);
+	  tp = get_thread_pointer (ptr_mode, true);
+	  dest = gen_rtx_PLUS (ptr_mode, tp, dest);
+	  if (GET_MODE (dest) != Pmode)
+	     dest = gen_rtx_ZERO_EXTEND (Pmode, dest);
 	  dest = force_reg (Pmode, dest);
 
 	  if (GET_MODE (x) != Pmode)
@@ -10815,7 +10806,7 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov)
       break;
 
     case TLS_MODEL_LOCAL_DYNAMIC:
-      base = gen_reg_rtx (Pmode);
+      base = gen_reg_rtx (TARGET_GNU2_TLS ? ptr_mode : Pmode);
 
       if (!TARGET_64BIT)
 	{
@@ -10833,13 +10824,19 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov)
 	  rtx tmp = ix86_tls_module_base ();
 
 	  if (TARGET_64BIT)
-	    emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, base, tmp));
+	    {
+	      emit_insn (gen_tls_dynamic_gnu2_64 (ptr_mode, base, tmp));
+	      if (GET_MODE (base) != Pmode)
+		base = gen_rtx_ZERO_EXTEND (Pmode, base);
+	    }
 	  else
 	    emit_insn (gen_tls_dynamic_gnu2_32 (base, tmp, pic));
 
-	  tp = get_thread_pointer (Pmode, true);
-	  set_unique_reg_note (get_last_insn (), REG_EQUAL,
-			       gen_rtx_MINUS (Pmode, tmp, tp));
+	  tp = get_thread_pointer (ptr_mode, true);
+	  tmp = gen_rtx_MINUS (ptr_mode, tmp, tp);
+	  if (GET_MODE (tmp) != Pmode)
+	    tmp = gen_rtx_ZERO_EXTEND (Pmode, tmp);
+	  set_unique_reg_note (get_last_insn (), REG_EQUAL, tmp);
 	}
       else
 	{
@@ -10876,17 +10873,14 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov)
 
       if (TARGET_GNU2_TLS)
 	{
-	  /* NB: Since DEST set by tls_dynamic_gnu2_64 is in ptr_mode,
-	     make sure that PLUS is done in ptr_mode.  */
-	  if (Pmode != ptr_mode)
+	  if (GET_MODE (tp) != Pmode)
 	    {
-	      tp = lowpart_subreg (ptr_mode, tp, Pmode);
 	      dest = lowpart_subreg (ptr_mode, dest, Pmode);
 	      dest = gen_rtx_PLUS (ptr_mode, tp, dest);
 	      dest = gen_rtx_ZERO_EXTEND (Pmode, dest);
 	    }
 	  else
-	    dest = gen_rtx_PLUS (Pmode, tp, dest);
+	    dest = gen_rtx_PLUS (ptr_mode, tp, dest);
 	  dest = force_reg (Pmode, dest);
 
 	  if (GET_MODE (x) != Pmode)
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index de335cb8f02..6c674aaea5b 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -15187,23 +15187,23 @@ (define_insn_and_split "*tls_dynamic_gnu2_combine_32"
 
 (define_expand "@tls_dynamic_gnu2_64_<mode>"
   [(set (match_dup 2)
-	(unspec:P [(match_operand 1 "tls_symbolic_operand")]
-		  UNSPEC_TLSDESC))
-   (parallel
-    [(set (match_operand:P 0 "register_operand")
-	  (unspec:P [(match_dup 1) (match_dup 2) (reg:P SP_REG)]
+	(unspec:PTR [(match_operand 1 "tls_symbolic_operand")]
 		    UNSPEC_TLSDESC))
+   (parallel
+    [(set (match_operand:PTR 0 "register_operand")
+	  (unspec:PTR [(match_dup 1) (match_dup 2) (reg:PTR SP_REG)]
+		      UNSPEC_TLSDESC))
      (clobber (reg:CC FLAGS_REG))])]
   "TARGET_64BIT && TARGET_GNU2_TLS"
 {
-  operands[2] = can_create_pseudo_p () ? gen_reg_rtx (Pmode) : operands[0];
+  operands[2] = can_create_pseudo_p () ? gen_reg_rtx (ptr_mode) : operands[0];
   ix86_tls_descriptor_calls_expanded_in_cfun = true;
 })
 
 (define_insn "*tls_dynamic_gnu2_lea_64_<mode>"
-  [(set (match_operand:P 0 "register_operand" "=r")
-	(unspec:P [(match_operand 1 "tls_symbolic_operand")]
-		  UNSPEC_TLSDESC))]
+  [(set (match_operand:PTR 0 "register_operand" "=r")
+	(unspec:PTR [(match_operand 1 "tls_symbolic_operand")]
+		    UNSPEC_TLSDESC))]
   "TARGET_64BIT && TARGET_GNU2_TLS"
   "lea%z0\t{%E1@TLSDESC(%%rip), %0|%0, %E1@TLSDESC[rip]}"
   [(set_attr "type" "lea")
@@ -15212,10 +15212,10 @@ (define_insn "*tls_dynamic_gnu2_lea_64_<mode>"
    (set_attr "length_address" "4")])
 
 (define_insn "*tls_dynamic_gnu2_call_64_<mode>"
-  [(set (match_operand:P 0 "register_operand" "=a")
-	(unspec:P [(match_operand 1 "tls_symbolic_operand")
-		   (match_operand:P 2 "register_operand" "0")
-		   (reg:P SP_REG)]
+  [(set (match_operand:PTR 0 "register_operand" "=a")
+	(unspec:PTR [(match_operand 1 "tls_symbolic_operand")
+		   (match_operand:PTR 2 "register_operand" "0")
+		   (reg:PTR SP_REG)]
 		  UNSPEC_TLSDESC))
    (clobber (reg:CC FLAGS_REG))]
   "TARGET_64BIT && TARGET_GNU2_TLS"
@@ -15225,23 +15225,23 @@ (define_insn "*tls_dynamic_gnu2_call_64_<mode>"
    (set_attr "length_address" "0")])
 
 (define_insn_and_split "*tls_dynamic_gnu2_combine_64_<mode>"
-  [(set (match_operand:P 0 "register_operand" "=&a")
-	(plus:P
-	 (unspec:P [(match_operand 2 "tls_modbase_operand")
-		     (match_operand:P 3)
-		     (reg:P SP_REG)]
-		   UNSPEC_TLSDESC)
-	 (const:P (unspec:P
-		    [(match_operand 1 "tls_symbolic_operand")]
-		    UNSPEC_DTPOFF))))
+  [(set (match_operand:PTR 0 "register_operand" "=&a")
+	(plus:PTR
+	 (unspec:PTR [(match_operand 2 "tls_modbase_operand")
+		      (match_operand:PTR 3)
+		      (reg:PTR SP_REG)]
+		     UNSPEC_TLSDESC)
+	 (const:PTR (unspec:PTR
+		     [(match_operand 1 "tls_symbolic_operand")]
+		     UNSPEC_DTPOFF))))
    (clobber (reg:CC FLAGS_REG))]
   "TARGET_64BIT && TARGET_GNU2_TLS"
   "#"
   ""
   [(set (match_dup 0) (match_dup 4))]
 {
-  operands[4] = can_create_pseudo_p () ? gen_reg_rtx (Pmode) : operands[0];
-  emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, operands[4], operands[1]));
+  operands[4] = can_create_pseudo_p () ? gen_reg_rtx (ptr_mode) : operands[0];
+  emit_insn (gen_tls_dynamic_gnu2_64 (ptr_mode, operands[4], operands[1]));
 })
 
 (define_split
-- 
2.24.1

Reply via email to