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. Uros.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 0b8a4b9ee4f0..ffe60baa72ad 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 de335cb8f029..6c674aaea5bf 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 8f6b4af3225e..122c111d0cbb 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; }