On Tue, Jan 21, 2020 at 2:29 AM Uros Bizjak <ubiz...@gmail.com> wrote:
>
> On Tue, Jan 21, 2020 at 9:47 AM Uros Bizjak <ubiz...@gmail.com> wrote:
> >
> > On Mon, Jan 20, 2020 at 10:46 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> >
> > > > > 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.
> >
> > case TLS_MODEL_GLOBAL_DYNAMIC:
> > - dest = gen_reg_rtx (Pmode);
> > + dest = gen_reg_rtx (TARGET_GNU2_TLS ? ptr_mode : Pmode);
> >
> > Please put these in their respective arms of "if (TARGET_GNU2_TLS).
> >
> > case TLS_MODEL_LOCAL_DYNAMIC:
> > - base = gen_reg_rtx (Pmode);
> > + base = gen_reg_rtx (TARGET_GNU2_TLS ? ptr_mode : Pmode);
> >
> > Also here.
> >
> > A question: Do we need to emit the following part in Pmode?
>
> To answer my own question: Yes. Linker doesn't like SImode relocs fox
> x86_64 and for
>
> addl $foo@dtpoff, %eax
>
> errors out with:
>
> pr93319-1a.s: Assembler messages:
> pr93319-1a.s:20: Error: relocated field and relocation type differ in
> signedness
>
> So, the part below is OK, except:
>
> - 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);
>
> I don't think we should attach this note to the thread pointer
> initialization. I have removed this part from the patch, but please
> review the decision.
>
> and
>
> - dest = gen_rtx_PLUS (Pmode, tp, dest);
> + dest = gen_rtx_PLUS (ptr_mode, tp, dest);
>
> Please leave Pmode here. ptr_mode == Pmode at this point, but Pmode
> better documents the mode selection logic.
>
> Also, the tests fail for me with:
>
> /usr/include/gnu/stubs.h:13:11: fatal error: gnu/stubs-x32.h: No such
> file or directory
>
> so either use __builtin_printf or some other approach that doesn't
> need to #include stdio.h.
>
> A patch that implements above changes is attached to the message.
>
Here is the updated patch. OK for master?
Thanks.
--
H.J.
From 01b20630518882fa3952962b26bfbb2465e08036 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] i386: Do GNU2 TLS address computation in ptr_mode
Since GNU2 TLS address from glibc run-time is in ptr_mode, we should do
GNU2 TLS address computation in ptr_mode and zero-extend result to Pmode.
2020-01-21 H.J. Lu <hongjiu...@intel.com>
Uros Bizjak <ubiz...@gmail.com>
gcc/
PR target/93319
* 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/testsuite/
2020-01-21 Uros Bizjak <ubiz...@gmail.com>
PR target/93319
* gcc.target/i386/pr93319-1a.c: Don't include <stdio.h>.
(test1): Replace printf with __builtin_printf.
---
gcc/config/i386/i386.c | 43 ++++++++-----------
gcc/config/i386/i386.md | 48 +++++++++++-----------
gcc/testsuite/gcc.target/i386/pr93319-1a.c | 6 +--
3 files changed, 42 insertions(+), 55 deletions(-)
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 0b8a4b9ee4f..ffe60baa72a 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,8 +10748,6 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov)
switch (model)
{
case TLS_MODEL_GLOBAL_DYNAMIC:
- dest = gen_reg_rtx (Pmode);
-
if (!TARGET_64BIT)
{
if (flag_pic && !TARGET_PECOFF)
@@ -10763,24 +10761,16 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov)
if (TARGET_GNU2_TLS)
{
+ dest = gen_reg_rtx (ptr_mode);
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)
@@ -10792,6 +10782,7 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov)
{
rtx caddr = ix86_tls_get_addr ();
+ dest = gen_reg_rtx (Pmode);
if (TARGET_64BIT)
{
rtx rax = gen_rtx_REG (Pmode, AX_REG);
@@ -10815,8 +10806,6 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov)
break;
case TLS_MODEL_LOCAL_DYNAMIC:
- base = gen_reg_rtx (Pmode);
-
if (!TARGET_64BIT)
{
if (flag_pic)
@@ -10832,19 +10821,22 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov)
{
rtx tmp = ix86_tls_module_base ();
+ base = gen_reg_rtx (ptr_mode);
if (TARGET_64BIT)
- emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, base, tmp));
+ emit_insn (gen_tls_dynamic_gnu2_64 (ptr_mode, base, tmp));
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);
+ if (GET_MODE (base) != Pmode)
+ base = gen_rtx_ZERO_EXTEND (Pmode, base);
+ base = force_reg (Pmode, base);
}
else
{
rtx caddr = ix86_tls_get_addr ();
+ base = gen_reg_rtx (Pmode);
if (TARGET_64BIT)
{
rtx rax = gen_rtx_REG (Pmode, AX_REG);
@@ -10876,11 +10868,8 @@ 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);
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_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 @@
(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 @@
(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
diff --git a/gcc/testsuite/gcc.target/i386/pr93319-1a.c b/gcc/testsuite/gcc.target/i386/pr93319-1a.c
index 8f6b4af3225..122c111d0cb 100644
--- a/gcc/testsuite/gcc.target/i386/pr93319-1a.c
+++ b/gcc/testsuite/gcc.target/i386/pr93319-1a.c
@@ -4,21 +4,19 @@
/* { dg-require-effective-target tls_native } */
/* { dg-options "-mx32 -fPIC -mtls-dialect=gnu2" } */
-#include <stdio.h>
-
extern __thread int bar;
static __thread int foo = 30;
int *
test1 (void)
{
- printf ("foo: %d\n", foo);
+ __builtin_printf ("foo: %d\n", foo);
return &foo;
}
int *
test2 (void)
{
- printf ("bar: %d\n", bar);
+ __builtin_printf ("bar: %d\n", bar);
return &bar;
}
--
2.24.1
