| Message ID | 20190815040510.GJ1969@bubble.grove.modra.org |
|---|---|
| State | New |
| Headers | show |
| Series | PC-relative TLS support | expand |
Hi! On Thu, Aug 15, 2019 at 01:35:10PM +0930, Alan Modra wrote: > Supporting TLS for -mpcrel turns out to be relatively simple, in part > due to deciding that !TARGET_TLS_MARKERS with -mpcrel is silly. No > assembler that I know of supporting prefix insns lacks TLS marker > support. Will this stay that way? (Or do we not care, not now anyway?) > Also, at some point powerpc gcc ought to remove > !TARGET_TLS_MARKERS generally and simplify all the occurrences of > IS_NOMARK_TLSGETADDR in rs6000.md rather than complicating them. The last time this came up (a year ago) the conclusion was that we first would have to remove AIX support. > * config/rs6000/predicates.md (unspec_tls): Allow const0_rtx for got > element of unspec vec. > * config/rs6000/rs6000.c (rs6000_option_override_internal): Disable > -mpcrel if -mno-tls-markers. > (rs6000_legitimize_tls_address): Support PC-relative TLS. > * config/rs6000/rs6000.md (UNSPEC_TLSTLS_PCREL): New unspec. > (tls_gd_pcrel, tls_ld_pcrel): New insns. > (tls_dtprel, tls_tprel): Set attr prefixed when tls_size is not 16. > (tls_got_tprel_pcrel, tls_tls_pcrel): New insns. (Changelog has whitespace damage, I guess that is just from how you mailed this? Please fix when applying it). The patch is fine when its prerequisites are in. Thanks, Segher
On Thu, Aug 15, 2019 at 01:24:07PM -0500, Segher Boessenkool wrote: > Hi! > > On Thu, Aug 15, 2019 at 01:35:10PM +0930, Alan Modra wrote: > > Supporting TLS for -mpcrel turns out to be relatively simple, in part > > due to deciding that !TARGET_TLS_MARKERS with -mpcrel is silly. No > > assembler that I know of supporting prefix insns lacks TLS marker > > support. > > Will this stay that way? (Or do we not care, not now anyway?) I'd say we leave that problem to someone who wants pcrel without tls markers. It's not hard to do, just extend rs6000_output_tlsargs and adjust IS_NOMARK_TLSGETADDR length attribute expressions. > > Also, at some point powerpc gcc ought to remove > > !TARGET_TLS_MARKERS generally and simplify all the occurrences of > > IS_NOMARK_TLSGETADDR in rs6000.md rather than complicating them. > > The last time this came up (a year ago) the conclusion was that we first > would have to remove AIX support. Hmm, I wonder has that changed? A quick look at the source says the AIX TLS support uses completely different patterns and shouldn't care. > (Changelog has whitespace damage, I guess that is just from how you > mailed this? Please fix when applying it). Fixed. (It wasn't the mailer..)
On Fri, Aug 16, 2019 at 11:29:30AM +0930, Alan Modra wrote: > On Thu, Aug 15, 2019 at 01:24:07PM -0500, Segher Boessenkool wrote: > > On Thu, Aug 15, 2019 at 01:35:10PM +0930, Alan Modra wrote: > > > Supporting TLS for -mpcrel turns out to be relatively simple, in part > > > due to deciding that !TARGET_TLS_MARKERS with -mpcrel is silly. No > > > assembler that I know of supporting prefix insns lacks TLS marker > > > support. > > > > Will this stay that way? (Or do we not care, not now anyway?) > > I'd say we leave that problem to someone who wants pcrel without tls > markers. It's not hard to do, just extend rs6000_output_tlsargs and > adjust IS_NOMARK_TLSGETADDR length attribute expressions. Okay, so the latter option :-) > > > Also, at some point powerpc gcc ought to remove > > > !TARGET_TLS_MARKERS generally and simplify all the occurrences of > > > IS_NOMARK_TLSGETADDR in rs6000.md rather than complicating them. > > > > The last time this came up (a year ago) the conclusion was that we first > > would have to remove AIX support. > > Hmm, I wonder has that changed? A quick look at the source says the > AIX TLS support uses completely different patterns and shouldn't care. https://gcc.gnu.org/ml/gcc-patches/2018-11/msg02259.html But if you think we can remove the !TARGET_TLS_MARKERS everywhere it is relevant at all, now is the time, patches very welcome, it would be a nice cleanup :-) Needs testing everywhere of course, but now is stage 1 :-) Segher
On Mon, Aug 19, 2019 at 07:45:19AM -0500, Segher Boessenkool wrote: > But if you think we can remove the !TARGET_TLS_MARKERS everywhere it > is relevant at all, now is the time, patches very welcome, it would be > a nice cleanup :-) Needs testing everywhere of course, but now is > stage 1 :-) This patch removes !TARGET_TLS_MARKERS support. -mtls-markers (and -mno-tls-markers) disappear as valid options too, because I figure they haven't been used too much except by people testing the compiler. Bootstrapped and regression tested powerpc64le-linux and powerpc-ibm-aix7.1.3.0 (on gcc111). I believe powerpc*-darwin doesn't support TLS. Requiring an 8 year old binutils-2.20 shouldn't be that onerous. Note that this patch doesn't remove the configure test to set HAVE_AS_TLS_MARKERS. I was wondering whether I ought to hook that into a "sorry, your assembler is too old" error? * config/rs6000/rs6000-protos.h (rs6000_output_tlsargs): Delete. * config/rs6000/rs6000.c (rs6000_output_tlsargs): Delete. (rs6000_legitimize_tls_address): Remove !TARGET_TLS_MARKERS code. (rs6000_call_template_1): Delete TARGET_TLS_MARKERS test and allow other UNSPECs besides UNSPEC_TLSGD and UNSPEC_TLSLD. (rs6000_indirect_call_template_1): Likewise. (rs6000_pltseq_template): Likewise. (rs6000_opt_vars): Remove "tls-markers" entry. * config/rs6000/rs6000.h (TARGET_TLS_MARKERS): Don't define. (IS_NOMARK_TLSGETADDR): Likewise. * config/rs6000/rs6000.md (tls_gd<bits>): Replace TARGET_TLS_MARKERS with !TARGET_XCOFF. (tls_gd_high<bits>, tls_gd_low<bits>): Likewise. (tls_ld<bits>, tls_ld_high<bits>, tls_ld_low<bits>): Likewise. (pltseq_plt_pcrel<mode>): Likewise. (call_value_local32): Remove IS_NOMARK_TLSGETADDR predicate test. (call_value_local64): Likewise. (call_value_indirect_nonlocal_sysv<mode>): Remove IS_NOMARK_TLSGETADDR output and length attribute sub-expression. (call_value_nonlocal_sysv<mode>), (call_value_nonlocal_sysv_secure<mode>), (call_value_local_aix<mode>, call_value_nonlocal_aix<mode>), (call_value_indirect_aix<mode>, call_value_indirect_elfv2<mode>), (call_value_indirect_pcrel<mode>): Likewise. * config/rs6000/rs6000.opt (mtls-markers): Delete. * doc/install.texi (powerpc-*-*): Require binutils-2.20. diff --git a/gcc/config/rs6000/rs6000-protos.h b/gcc/config/rs6000/rs6000-protos.h index 06e40d94b17..88b5b7cec55 100644 --- a/gcc/config/rs6000/rs6000-protos.h +++ b/gcc/config/rs6000/rs6000-protos.h @@ -139,7 +139,6 @@ extern bool valid_sf_si_move (rtx, rtx, machine_mode); extern void rs6000_emit_move (rtx, rtx, machine_mode); extern bool rs6000_legitimate_offset_address_p (machine_mode, rtx, bool, bool); -extern void rs6000_output_tlsargs (rtx *); extern rtx rs6000_find_base_term (rtx); extern rtx rs6000_return_addr (int, rtx); extern void rs6000_output_symbol_ref (FILE*, rtx); diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index e792116fb40..5e2b08c3c72 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -8329,41 +8329,6 @@ rs6000_legitimize_tls_address_aix (rtx addr, enum tls_model model) return dest; } -/* Output arg setup instructions for a !TARGET_TLS_MARKERS - __tls_get_addr call. */ - -void -rs6000_output_tlsargs (rtx *operands) -{ - /* Set up operands for output_asm_insn, without modifying OPERANDS. */ - rtx op[3]; - - /* The set dest of the call, ie. r3, which is also the first arg reg. */ - op[0] = operands[0]; - /* The TLS symbol from global_tlsarg stashed as CALL operand 2. */ - op[1] = XVECEXP (operands[2], 0, 0); - if (XINT (operands[2], 1) == UNSPEC_TLSGD) - { - /* The GOT register. */ - op[2] = XVECEXP (operands[2], 0, 1); - if (TARGET_CMODEL != CMODEL_SMALL) - output_asm_insn ("addis %0,%2,%1@got@tlsgd@ha\n\t" - "addi %0,%0,%1@got@tlsgd@l", op); - else - output_asm_insn ("addi %0,%2,%1@got@tlsgd", op); - } - else if (XINT (operands[2], 1) == UNSPEC_TLSLD) - { - if (TARGET_CMODEL != CMODEL_SMALL) - output_asm_insn ("addis %0,%1,%&@got@tlsld@ha\n\t" - "addi %0,%0,%&@got@tlsld@l", op); - else - output_asm_insn ("addi %0,%1,%&@got@tlsld", op); - } - else - gcc_unreachable (); -} - /* Passes the tls arg value for global dynamic and local dynamic emit_library_call_value in rs6000_legitimize_tls_address to rs6000_call_aix and rs6000_call_sysv. This is used to emit the @@ -8465,16 +8430,10 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model model) rtx arg = gen_rtx_UNSPEC (Pmode, gen_rtvec (2, addr, got), UNSPEC_TLSGD); tga = rs6000_tls_get_addr (); + rtx argreg = gen_rtx_REG (Pmode, 3); + emit_insn (gen_rtx_SET (argreg, arg)); global_tlsarg = arg; - if (TARGET_TLS_MARKERS) - { - rtx argreg = gen_rtx_REG (Pmode, 3); - emit_insn (gen_rtx_SET (argreg, arg)); - emit_library_call_value (tga, dest, LCT_CONST, Pmode, - argreg, Pmode); - } - else - emit_library_call_value (tga, dest, LCT_CONST, Pmode); + emit_library_call_value (tga, dest, LCT_CONST, Pmode, argreg, Pmode); global_tlsarg = NULL_RTX; /* Make a note so that the result of this call can be CSEd. */ @@ -8487,16 +8446,10 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model model) rtx arg = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, got), UNSPEC_TLSLD); tga = rs6000_tls_get_addr (); tmp1 = gen_reg_rtx (Pmode); + rtx argreg = gen_rtx_REG (Pmode, 3); + emit_insn (gen_rtx_SET (argreg, arg)); global_tlsarg = arg; - if (TARGET_TLS_MARKERS) - { - rtx argreg = gen_rtx_REG (Pmode, 3); - emit_insn (gen_rtx_SET (argreg, arg)); - emit_library_call_value (tga, tmp1, LCT_CONST, Pmode, - argreg, Pmode); - } - else - emit_library_call_value (tga, tmp1, LCT_CONST, Pmode); + emit_library_call_value (tga, tmp1, LCT_CONST, Pmode, argreg, Pmode); global_tlsarg = NULL_RTX; /* Make a note so that the result of this call can be CSEd. */ @@ -13270,14 +13223,12 @@ rs6000_call_template_1 (rtx *operands, unsigned int funop, bool sibcall) char arg[12]; arg[0] = 0; - if (TARGET_TLS_MARKERS && GET_CODE (operands[funop + 1]) == UNSPEC) + if (GET_CODE (operands[funop + 1]) == UNSPEC) { if (XINT (operands[funop + 1], 1) == UNSPEC_TLSGD) sprintf (arg, "(%%%u@tlsgd)", funop + 1); else if (XINT (operands[funop + 1], 1) == UNSPEC_TLSLD) sprintf (arg, "(%%&@tlsld)"); - else - gcc_unreachable (); } /* The magic 32768 offset here corresponds to the offset of @@ -13418,7 +13369,7 @@ rs6000_indirect_call_template_1 (rtx *operands, unsigned int funop, const char *rel64 = TARGET_64BIT ? "64" : ""; char tls[29]; tls[0] = 0; - if (TARGET_TLS_MARKERS && GET_CODE (operands[funop + 1]) == UNSPEC) + if (GET_CODE (operands[funop + 1]) == UNSPEC) { if (XINT (operands[funop + 1], 1) == UNSPEC_TLSGD) sprintf (tls, ".reloc .,R_PPC%s_TLSGD,%%%u\n\t", @@ -13426,8 +13377,6 @@ rs6000_indirect_call_template_1 (rtx *operands, unsigned int funop, else if (XINT (operands[funop + 1], 1) == UNSPEC_TLSLD) sprintf (tls, ".reloc .,R_PPC%s_TLSLD,%%&\n\t", rel64); - else - gcc_unreachable (); } const char *notoc = rs6000_pcrel_p (cfun) ? "_NOTOC" : ""; @@ -13514,7 +13463,7 @@ rs6000_pltseq_template (rtx *operands, int which) const char *rel64 = TARGET_64BIT ? "64" : ""; char tls[30]; tls[0] = 0; - if (TARGET_TLS_MARKERS && GET_CODE (operands[3]) == UNSPEC) + if (GET_CODE (operands[3]) == UNSPEC) { char off = which == RS6000_PLTSEQ_PLT_PCREL34 ? '8' : '4'; if (XINT (operands[3], 1) == UNSPEC_TLSGD) @@ -13523,8 +13472,6 @@ rs6000_pltseq_template (rtx *operands, int which) else if (XINT (operands[3], 1) == UNSPEC_TLSLD) sprintf (tls, ".reloc .-%c,R_PPC%s_TLSLD,%%&\n\t", off, rel64); - else - gcc_unreachable (); } gcc_assert (DEFAULT_ABI == ABI_ELFv2 || DEFAULT_ABI == ABI_V4); @@ -22866,9 +22813,6 @@ static struct rs6000_opt_var const rs6000_opt_vars[] = { "align-branch-targets", offsetof (struct gcc_options, x_TARGET_ALIGN_BRANCH_TARGETS), offsetof (struct cl_target_option, x_TARGET_ALIGN_BRANCH_TARGETS), }, - { "tls-markers", - offsetof (struct gcc_options, x_tls_markers), - offsetof (struct cl_target_option, x_tls_markers), }, { "sched-prolog", offsetof (struct gcc_options, x_TARGET_SCHED_PROLOG), offsetof (struct cl_target_option, x_TARGET_SCHED_PROLOG), }, diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h index 9c11a3e4d46..b263213ad75 100644 --- a/gcc/config/rs6000/rs6000.h +++ b/gcc/config/rs6000/rs6000.h @@ -228,15 +228,6 @@ extern const char *host_detect_local_cpu (int argc, const char **argv); #define TARGET_MFCRF 0 #endif -/* Define TARGET_TLS_MARKERS if the target assembler does not support - arg markers for __tls_get_addr calls. */ -#ifndef HAVE_AS_TLS_MARKERS -#undef TARGET_TLS_MARKERS -#define TARGET_TLS_MARKERS 0 -#else -#define TARGET_TLS_MARKERS tls_markers -#endif - #ifndef TARGET_SECURE_PLT #define TARGET_SECURE_PLT 0 #endif @@ -1513,13 +1504,6 @@ enum rs6000_pltseq_enum { #define IS_V4_FP_ARGS(OP) \ ((INTVAL (OP) & (CALL_V4_CLEAR_FP_ARGS | CALL_V4_SET_FP_ARGS)) != 0) -/* Whether OP is an UNSPEC used in !TARGET_TLS_MARKER calls. */ -#define IS_NOMARK_TLSGETADDR(OP) \ - (!TARGET_TLS_MARKERS \ - && GET_CODE (OP) == UNSPEC \ - && (XINT (OP, 1) == UNSPEC_TLSGD \ - || XINT (OP, 1) == UNSPEC_TLSLD)) - /* We don't have prologue and epilogue functions to save/restore everything for most ABIs. */ #define WORLD_SAVE_P(INFO) 0 diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 9a7a1da987f..b5b4bc1587e 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -9413,7 +9413,7 @@ (define_insn_and_split "*tls_gd<bits>" (unspec:P [(match_operand:P 1 "rs6000_tls_symbol_ref" "") (match_operand:P 2 "gpc_reg_operand" "b")] UNSPEC_TLSGD))] - "HAVE_AS_TLS && TARGET_TLS_MARKERS" + "HAVE_AS_TLS && !TARGET_XCOFF" "addi %0,%2,%1@got@tlsgd" "&& TARGET_CMODEL != CMODEL_SMALL" [(set (match_dup 3) @@ -9436,7 +9436,7 @@ (define_insn "*tls_gd_high<bits>" (unspec:P [(match_operand:P 1 "rs6000_tls_symbol_ref" "") (match_operand:P 2 "gpc_reg_operand" "b")] UNSPEC_TLSGD)))] - "HAVE_AS_TLS && TARGET_TLS_MARKERS && TARGET_CMODEL != CMODEL_SMALL" + "HAVE_AS_TLS && !TARGET_XCOFF && TARGET_CMODEL != CMODEL_SMALL" "addis %0,%2,%1@got@tlsgd@ha") (define_insn "*tls_gd_low<bits>" @@ -9445,14 +9445,14 @@ (define_insn "*tls_gd_low<bits>" (unspec:P [(match_operand:P 2 "rs6000_tls_symbol_ref" "") (match_operand:P 3 "gpc_reg_operand" "b")] UNSPEC_TLSGD)))] - "HAVE_AS_TLS && TARGET_TLS_MARKERS && TARGET_CMODEL != CMODEL_SMALL" + "HAVE_AS_TLS && !TARGET_XCOFF && TARGET_CMODEL != CMODEL_SMALL" "addi %0,%1,%2@got@tlsgd@l") (define_insn_and_split "*tls_ld<bits>" [(set (match_operand:P 0 "gpc_reg_operand" "=b") (unspec:P [(match_operand:P 1 "gpc_reg_operand" "b")] UNSPEC_TLSLD))] - "HAVE_AS_TLS && TARGET_TLS_MARKERS" + "HAVE_AS_TLS && !TARGET_XCOFF" "addi %0,%1,%&@got@tlsld" "&& TARGET_CMODEL != CMODEL_SMALL" [(set (match_dup 2) @@ -9474,7 +9474,7 @@ (define_insn "*tls_ld_high<bits>" (high:P (unspec:P [(match_operand:P 1 "gpc_reg_operand" "b")] UNSPEC_TLSLD)))] - "HAVE_AS_TLS && TARGET_TLS_MARKERS && TARGET_CMODEL != CMODEL_SMALL" + "HAVE_AS_TLS && !TARGET_XCOFF && TARGET_CMODEL != CMODEL_SMALL" "addis %0,%1,%&@got@tlsld@ha") (define_insn "*tls_ld_low<bits>" @@ -9482,7 +9482,7 @@ (define_insn "*tls_ld_low<bits>" (lo_sum:P (match_operand:P 1 "gpc_reg_operand" "b") (unspec:P [(match_operand:P 2 "gpc_reg_operand" "b")] UNSPEC_TLSLD)))] - "HAVE_AS_TLS && TARGET_TLS_MARKERS && TARGET_CMODEL != CMODEL_SMALL" + "HAVE_AS_TLS && !TARGET_XCOFF && TARGET_CMODEL != CMODEL_SMALL" "addi %0,%1,%&@got@tlsld@l") (define_insn "tls_dtprel_<bits>" @@ -10193,7 +10193,7 @@ (define_insn "*pltseq_plt_pcrel<mode>" (match_operand:P 2 "symbol_ref_operand" "s") (match_operand:P 3 "" "")] UNSPEC_PLT_PCREL))] - "HAVE_AS_PLTSEQ && TARGET_TLS_MARKERS + "HAVE_AS_PLTSEQ && !TARGET_XCOFF && rs6000_pcrel_p (cfun)" { return rs6000_pltseq_template (operands, RS6000_PLTSEQ_PLT_PCREL34); @@ -10308,8 +10308,7 @@ (define_insn "*call_value_local32" (match_operand 2))) (use (match_operand:SI 3 "immediate_operand" "O,n")) (clobber (reg:SI LR_REGNO))] - "(INTVAL (operands[3]) & CALL_LONG) == 0 - && !IS_NOMARK_TLSGETADDR (operands[2])" + "(INTVAL (operands[3]) & CALL_LONG) == 0" { if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS) output_asm_insn ("crxor 6,6,6", operands); @@ -10329,8 +10328,7 @@ (define_insn "*call_value_local64" (match_operand 2))) (use (match_operand:SI 3 "immediate_operand" "O,n")) (clobber (reg:DI LR_REGNO))] - "TARGET_64BIT && (INTVAL (operands[3]) & CALL_LONG) == 0 - && !IS_NOMARK_TLSGETADDR (operands[2])" + "TARGET_64BIT && (INTVAL (operands[3]) & CALL_LONG) == 0" { if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS) output_asm_insn ("crxor 6,6,6", operands); @@ -10428,10 +10426,7 @@ (define_insn "*call_value_indirect_nonlocal_sysv<mode>" "DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_DARWIN" { - if (IS_NOMARK_TLSGETADDR (operands[2])) - rs6000_output_tlsargs (operands); - - else if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS) + if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS) output_asm_insn ("crxor 6,6,6", operands); else if (INTVAL (operands[3]) & CALL_V4_CLEAR_FP_ARGS) @@ -10442,8 +10437,7 @@ (define_insn "*call_value_indirect_nonlocal_sysv<mode>" [(set_attr "type" "jmpreg") (set (attr "length") (plus - (if_then_else (ior (match_test "IS_NOMARK_TLSGETADDR (operands[2])") - (match_test "IS_V4_FP_ARGS (operands[3])")) + (if_then_else (match_test "IS_V4_FP_ARGS (operands[3])") (const_int 4) (const_int 0)) (if_then_else (and (match_test "!rs6000_speculate_indirect_jumps") @@ -10461,10 +10455,7 @@ (define_insn "*call_value_nonlocal_sysv<mode>" || (DEFAULT_ABI == ABI_V4 && (INTVAL (operands[3]) & CALL_LONG) == 0))" { - if (IS_NOMARK_TLSGETADDR (operands[2])) - rs6000_output_tlsargs (operands); - - else if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS) + if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS) output_asm_insn ("crxor 6,6,6", operands); else if (INTVAL (operands[3]) & CALL_V4_CLEAR_FP_ARGS) @@ -10474,8 +10465,7 @@ (define_insn "*call_value_nonlocal_sysv<mode>" } [(set_attr "type" "branch") (set (attr "length") - (if_then_else (ior (match_test "IS_NOMARK_TLSGETADDR (operands[2])") - (match_test "IS_V4_FP_ARGS (operands[3])")) + (if_then_else (match_test "IS_V4_FP_ARGS (operands[3])") (const_int 8) (const_int 4)))]) @@ -10490,10 +10480,7 @@ (define_insn "*call_value_nonlocal_sysv_secure<mode>" && TARGET_SECURE_PLT && flag_pic && !SYMBOL_REF_LOCAL_P (operands[1]) && (INTVAL (operands[3]) & CALL_LONG) == 0)" { - if (IS_NOMARK_TLSGETADDR (operands[2])) - rs6000_output_tlsargs (operands); - - else if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS) + if (INTVAL (operands[3]) & CALL_V4_SET_FP_ARGS) output_asm_insn ("crxor 6,6,6", operands); else if (INTVAL (operands[3]) & CALL_V4_CLEAR_FP_ARGS) @@ -10503,8 +10490,7 @@ (define_insn "*call_value_nonlocal_sysv_secure<mode>" } [(set_attr "type" "branch") (set (attr "length") - (if_then_else (ior (match_test "IS_NOMARK_TLSGETADDR (operands[2])") - (match_test "IS_V4_FP_ARGS (operands[3])")) + (if_then_else (match_test "IS_V4_FP_ARGS (operands[3])") (const_int 8) (const_int 4)))]) @@ -10527,8 +10513,7 @@ (define_insn "*call_value_local_aix<mode>" (call (mem:SI (match_operand:P 1 "current_file_function_operand" "s")) (match_operand 2))) (clobber (reg:P LR_REGNO))] - "(DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2) - && !IS_NOMARK_TLSGETADDR (operands[2])" + "DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2" { if (rs6000_pcrel_p (cfun)) return "bl %z1@notoc"; @@ -10560,21 +10545,13 @@ (define_insn "*call_value_nonlocal_aix<mode>" (clobber (reg:P LR_REGNO))] "DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2" { - if (IS_NOMARK_TLSGETADDR (operands[2])) - rs6000_output_tlsargs (operands); - return rs6000_call_template (operands, 1); } [(set_attr "type" "branch") (set (attr "length") - (plus (if_then_else (match_test "IS_NOMARK_TLSGETADDR (operands[2])") - (if_then_else (match_test "TARGET_CMODEL != CMODEL_SMALL") - (const_int 8) - (const_int 4)) - (const_int 0)) - (if_then_else (match_test "rs6000_pcrel_p (cfun)") - (const_int 4) - (const_int 8))))]) + (if_then_else (match_test "rs6000_pcrel_p (cfun)") + (const_int 4) + (const_int 8)))]) ;; Call to indirect functions with the AIX abi using a 3 word descriptor. ;; Operand0 is the addresss of the function to call @@ -10609,23 +10586,14 @@ (define_insn "*call_value_indirect_aix<mode>" (clobber (reg:P LR_REGNO))] "DEFAULT_ABI == ABI_AIX" { - if (IS_NOMARK_TLSGETADDR (operands[2])) - rs6000_output_tlsargs (operands); - return rs6000_indirect_call_template (operands, 1); } [(set_attr "type" "jmpreg") (set (attr "length") - (plus - (if_then_else (match_test "IS_NOMARK_TLSGETADDR (operands[2])") - (if_then_else (match_test "TARGET_CMODEL != CMODEL_SMALL") - (const_int 8) - (const_int 4)) - (const_int 0)) - (if_then_else (and (match_test "!rs6000_speculate_indirect_jumps") - (match_test "which_alternative != 1")) + (if_then_else (and (match_test "!rs6000_speculate_indirect_jumps") + (match_test "which_alternative != 1")) (const_string "16") - (const_string "12"))))]) + (const_string "12")))]) ;; Call to indirect functions with the ELFv2 ABI. ;; Operand0 is the addresss of the function to call @@ -10672,23 +10640,14 @@ (define_insn "*call_value_indirect_elfv2<mode>" (clobber (reg:P LR_REGNO))] "DEFAULT_ABI == ABI_ELFv2" { - if (IS_NOMARK_TLSGETADDR (operands[2])) - rs6000_output_tlsargs (operands); - return rs6000_indirect_call_template (operands, 1); } [(set_attr "type" "jmpreg") (set (attr "length") - (plus - (if_then_else (match_test "IS_NOMARK_TLSGETADDR (operands[2])") - (if_then_else (match_test "TARGET_CMODEL != CMODEL_SMALL") - (const_int 8) - (const_int 4)) - (const_int 0)) - (if_then_else (and (match_test "!rs6000_speculate_indirect_jumps") - (match_test "which_alternative != 1")) + (if_then_else (and (match_test "!rs6000_speculate_indirect_jumps") + (match_test "which_alternative != 1")) (const_string "12") - (const_string "8"))))]) + (const_string "8")))]) (define_insn "*call_value_indirect_pcrel<mode>" [(set (match_operand 0 "" "") @@ -10697,23 +10656,14 @@ (define_insn "*call_value_indirect_pcrel<mode>" (clobber (reg:P LR_REGNO))] "rs6000_pcrel_p (cfun)" { - if (IS_NOMARK_TLSGETADDR (operands[2])) - rs6000_output_tlsargs (operands); - return rs6000_indirect_call_template (operands, 1); } [(set_attr "type" "jmpreg") (set (attr "length") - (plus - (if_then_else (match_test "IS_NOMARK_TLSGETADDR (operands[2])") - (if_then_else (match_test "TARGET_CMODEL != CMODEL_SMALL") - (const_int 8) - (const_int 4)) - (const_int 0)) - (if_then_else (and (match_test "!rs6000_speculate_indirect_jumps") - (match_test "which_alternative != 1")) + (if_then_else (and (match_test "!rs6000_speculate_indirect_jumps") + (match_test "which_alternative != 1")) (const_string "8") - (const_string "4"))))]) + (const_string "4")))]) ;; Call subroutine returning any type. (define_expand "untyped_call" diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt index 1b69507cfa8..29803b753eb 100644 --- a/gcc/config/rs6000/rs6000.opt +++ b/gcc/config/rs6000/rs6000.opt @@ -246,10 +246,6 @@ mavoid-indexed-addresses Target Report Var(TARGET_AVOID_XFORM) Init(-1) Save Avoid generation of indexed load/store instructions when possible. -mtls-markers -Target Report Var(tls_markers) Init(1) Save -Mark __tls_get_addr calls with argument info. - msched-epilog Target Undocumented Var(TARGET_SCHED_PROLOG) Init(1) Save diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi index df6fefd72b9..feeda9fcc0e 100644 --- a/gcc/doc/install.texi +++ b/gcc/doc/install.texi @@ -4354,7 +4354,7 @@ The OpenRISC 1000 32-bit processor with delay slots. You can specify a default version for the @option{-mcpu=@var{cpu_type}} switch by using the configure option @option{--with-cpu-@var{cpu_type}}. -You will need GNU binutils 2.15 or newer. +You will need GNU binutils 2.20 or newer. @html <hr />
On Wed, Aug 21, 2019 at 09:55:28PM +0930, Alan Modra wrote: > On Mon, Aug 19, 2019 at 07:45:19AM -0500, Segher Boessenkool wrote: > > But if you think we can remove the !TARGET_TLS_MARKERS everywhere it > > is relevant at all, now is the time, patches very welcome, it would be > > a nice cleanup :-) Needs testing everywhere of course, but now is > > stage 1 :-) > > This patch removes !TARGET_TLS_MARKERS support. -mtls-markers (and > -mno-tls-markers) disappear as valid options too, because I figure > they haven't been used too much except by people testing the > compiler. Bootstrapped and regression tested powerpc64le-linux and > powerpc-ibm-aix7.1.3.0 (on gcc111). I believe powerpc*-darwin doesn't > support TLS. > > Requiring an 8 year old binutils-2.20 shouldn't be that onerous. > > Note that this patch doesn't remove the configure test to set > HAVE_AS_TLS_MARKERS. I was wondering whether I ought to hook that > into a "sorry, your assembler is too old" error? https://gcc.gnu.org/ml/gcc-patches/2019-08/msg01487.html I should have pinged this before now, and really I think the following additional patch makes more sense than any sort of sorry message. Mostly people will be running the assembler anyway so will discover quickly that their assembler is too old. * configure.ac (HAVE_AS_TLS_MARKERS): Delete test. * configure: Regenerate. * config.in: Regenerate. diff --git a/gcc/configure.ac b/gcc/configure.ac index 5f32fd4d5e4..44d816630e9 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -4811,12 +4811,6 @@ LCF0: [AC_DEFINE(HAVE_AS_GNU_ATTRIBUTE, 1, [Define if your assembler supports .gnu_attribute.])]) - gcc_GAS_CHECK_FEATURE([tls marker support], - gcc_cv_as_powerpc_tls_markers, [2,20,0],, - [ bl __tls_get_addr(x@tlsgd)],, - [AC_DEFINE(HAVE_AS_TLS_MARKERS, 1, - [Define if your assembler supports arg info for __tls_get_addr.])]) - gcc_GAS_CHECK_FEATURE([prologue entry point marker support], gcc_cv_as_powerpc_entry_markers, [2,26,0],-a64 --fatal-warnings, [ .reloc .,R_PPC64_ENTRY; nop],, > * config/rs6000/rs6000-protos.h (rs6000_output_tlsargs): Delete. > * config/rs6000/rs6000.c (rs6000_output_tlsargs): Delete. > (rs6000_legitimize_tls_address): Remove !TARGET_TLS_MARKERS code. > (rs6000_call_template_1): Delete TARGET_TLS_MARKERS test and > allow other UNSPECs besides UNSPEC_TLSGD and UNSPEC_TLSLD. > (rs6000_indirect_call_template_1): Likewise. > (rs6000_pltseq_template): Likewise. > (rs6000_opt_vars): Remove "tls-markers" entry. > * config/rs6000/rs6000.h (TARGET_TLS_MARKERS): Don't define. > (IS_NOMARK_TLSGETADDR): Likewise. > * config/rs6000/rs6000.md (tls_gd<bits>): Replace TARGET_TLS_MARKERS > with !TARGET_XCOFF. > (tls_gd_high<bits>, tls_gd_low<bits>): Likewise. > (tls_ld<bits>, tls_ld_high<bits>, tls_ld_low<bits>): Likewise. > (pltseq_plt_pcrel<mode>): Likewise. > (call_value_local32): Remove IS_NOMARK_TLSGETADDR predicate test. > (call_value_local64): Likewise. > (call_value_indirect_nonlocal_sysv<mode>): Remove IS_NOMARK_TLSGETADDR > output and length attribute sub-expression. > (call_value_nonlocal_sysv<mode>), > (call_value_nonlocal_sysv_secure<mode>), > (call_value_local_aix<mode>, call_value_nonlocal_aix<mode>), > (call_value_indirect_aix<mode>, call_value_indirect_elfv2<mode>), > (call_value_indirect_pcrel<mode>): Likewise. > * config/rs6000/rs6000.opt (mtls-markers): Delete. > * doc/install.texi (powerpc-*-*): Require binutils-2.20.
Hi Alan, On Mon, Nov 11, 2019 at 05:46:01PM +1030, Alan Modra wrote: > On Wed, Aug 21, 2019 at 09:55:28PM +0930, Alan Modra wrote: > > On Mon, Aug 19, 2019 at 07:45:19AM -0500, Segher Boessenkool wrote: > > > But if you think we can remove the !TARGET_TLS_MARKERS everywhere it > > > is relevant at all, now is the time, patches very welcome, it would be > > > a nice cleanup :-) Needs testing everywhere of course, but now is > > > stage 1 :-) > > > > This patch removes !TARGET_TLS_MARKERS support. -mtls-markers (and > > -mno-tls-markers) disappear as valid options too, because I figure > > they haven't been used too much except by people testing the > > compiler. Bootstrapped and regression tested powerpc64le-linux and > > powerpc-ibm-aix7.1.3.0 (on gcc111). I believe powerpc*-darwin doesn't > > support TLS. Excellent :-) > > Requiring an 8 year old binutils-2.20 shouldn't be that onerous. Right. Your binutils should be about the same vintage as your GCC, or various things will not work anyway. And building binutils is easy and cheap anyway, compared to building GCC. > I should have pinged this before now, And I shouldn't have dropped it :-) > and really I think the following > additional patch makes more sense than any sort of sorry message. > Mostly people will be running the assembler anyway so will discover > quickly that their assembler is too old. Yeah, that's fine. Almost no one would ever hit that anwyay. I'll reply to the rest as reply to the original mail. Segher
On Wed, Aug 21, 2019 at 09:55:28PM +0930, Alan Modra wrote: > This patch removes !TARGET_TLS_MARKERS support. -mtls-markers (and > -mno-tls-markers) disappear as valid options too, because I figure > they haven't been used too much except by people testing the > compiler. Okay. > (rs6000_call_template_1): Delete TARGET_TLS_MARKERS test and > allow other UNSPECs besides UNSPEC_TLSGD and UNSPEC_TLSLD. Why is that? Should we allow the other code that can happen and keep the gcc_unreachable? Or do we know that no other code can happen here ever, and the extra documentation isn't useful? > --- a/gcc/config/rs6000/rs6000.md > +++ b/gcc/config/rs6000/rs6000.md > @@ -9413,7 +9413,7 @@ (define_insn_and_split "*tls_gd<bits>" > (unspec:P [(match_operand:P 1 "rs6000_tls_symbol_ref" "") > (match_operand:P 2 "gpc_reg_operand" "b")] > UNSPEC_TLSGD))] > - "HAVE_AS_TLS && TARGET_TLS_MARKERS" > + "HAVE_AS_TLS && !TARGET_XCOFF" Should that be TARGET_ELF instead? Okay for trunk with those two things looked at. Thanks! Segher
On Mon, Nov 11, 2019 at 05:56:47AM -0600, Segher Boessenkool wrote: > On Wed, Aug 21, 2019 at 09:55:28PM +0930, Alan Modra wrote: > > This patch removes !TARGET_TLS_MARKERS support. -mtls-markers (and > > -mno-tls-markers) disappear as valid options too, because I figure > > they haven't been used too much except by people testing the > > compiler. > > Okay. > > > (rs6000_call_template_1): Delete TARGET_TLS_MARKERS test and > > allow other UNSPECs besides UNSPEC_TLSGD and UNSPEC_TLSLD. > > Why is that? Should we allow the other code that can happen and keep > the gcc_unreachable? Or do we know that no other code can happen here > ever, and the extra documentation isn't useful? The code in question is just printing the @tlsgd or @tlsld arg. I don't see any point in asserting that no other UNSPEC could ever be used in a call operand. Other places dealing with UNSPEC_TLSGD and UNSPEC_TLSLD don't check, and if another UNSPEC is invented for some fancy future call insn it's quite unlikely to want to output anything here. (I don't think I found such an UNSPEC already extant..) > > --- a/gcc/config/rs6000/rs6000.md > > +++ b/gcc/config/rs6000/rs6000.md > > @@ -9413,7 +9413,7 @@ (define_insn_and_split "*tls_gd<bits>" > > (unspec:P [(match_operand:P 1 "rs6000_tls_symbol_ref" "") > > (match_operand:P 2 "gpc_reg_operand" "b")] > > UNSPEC_TLSGD))] > > - "HAVE_AS_TLS && TARGET_TLS_MARKERS" > > + "HAVE_AS_TLS && !TARGET_XCOFF" > > Should that be TARGET_ELF instead? Either should work. So, yes, probably better with TARGET_ELF.
diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md index fba87946ec7..4ea588e1027 100644 --- a/gcc/config/rs6000/predicates.md +++ b/gcc/config/rs6000/predicates.md @@ -995,9 +995,9 @@ if (CONST_INT_P (op)) return 1; if (XINT (op, 1) == UNSPEC_TLSGD) - return REG_P (XVECEXP (op, 0, 1)); + return REG_P (XVECEXP (op, 0, 1)) || XVECEXP (op, 0, 1) == const0_rtx; if (XINT (op, 1) == UNSPEC_TLSLD) - return REG_P (XVECEXP (op, 0, 0)); + return REG_P (XVECEXP (op, 0, 0)) || XVECEXP (op, 0, 0) == const0_rtx; return 0; }) diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 6aca0ce5bf3..c04206ab139 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -4216,6 +4216,16 @@ rs6000_option_override_internal (bool global_init_p) | OPTION_MASK_PCREL_OPT); } + /* -mpcrel requires tls marker support. */ + if (TARGET_PCREL && !TARGET_TLS_MARKERS) + { + if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0) + error ("%qs requires %qs", "-mpcrel", "-mtls-markers"); + + rs6000_isa_flags &= ~(OPTION_MASK_PCREL + | OPTION_MASK_PCREL_OPT); + } + /* Check -mfuture debug switches. */ if (!TARGET_PCREL && TARGET_PCREL_OPT) { @@ -8613,7 +8623,8 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model model) return rs6000_legitimize_tls_address_aix (addr, model); dest = gen_reg_rtx (Pmode); - if (model == TLS_MODEL_LOCAL_EXEC && rs6000_tls_size == 16) + if (model == TLS_MODEL_LOCAL_EXEC + && (rs6000_tls_size == 16 || rs6000_pcrel_p (cfun))) { rtx tlsreg; @@ -8660,7 +8671,9 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model model) them in the .got section. So use a pointer to the .got section, not one to secondary TOC sections used by 64-bit -mminimal-toc, or to secondary GOT sections used by 32-bit -fPIC. */ - if (TARGET_64BIT) + if (rs6000_pcrel_p (cfun)) + got = const0_rtx; + else if (TARGET_64BIT) got = gen_rtx_REG (Pmode, 2); else { @@ -8735,7 +8748,7 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model model) rtx uns = gen_rtx_UNSPEC (Pmode, vec, UNSPEC_TLS_GET_ADDR); set_unique_reg_note (get_last_insn (), REG_EQUAL, uns); - if (rs6000_tls_size == 16) + if (rs6000_tls_size == 16 || rs6000_pcrel_p (cfun)) { if (TARGET_64BIT) insn = gen_tls_dtprel_64 (dest, tmp1, addr); @@ -8776,7 +8789,14 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model model) else insn = gen_tls_got_tprel_32 (tmp2, got, addr); emit_insn (insn); - if (TARGET_64BIT) + if (rs6000_pcrel_p (cfun)) + { + if (TARGET_64BIT) + insn = gen_tls_tls_pcrel_64 (dest, tmp2, addr); + else + insn = gen_tls_tls_pcrel_32 (dest, tmp2, addr); + } + else if (TARGET_64BIT) insn = gen_tls_tls_64 (dest, tmp2, addr); else insn = gen_tls_tls_32 (dest, tmp2, addr); diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 0e7d90e5357..6e32d8fdff1 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -88,6 +88,7 @@ UNSPEC_TLSTPRELLO UNSPEC_TLSGOTTPREL UNSPEC_TLSTLS + UNSPEC_TLSTLS_PCREL UNSPEC_FIX_TRUNC_TF ; fadd, rounding towards zero UNSPEC_STFIWX UNSPEC_POPCNTB @@ -9514,6 +9515,15 @@ ;; TLS support. +(define_insn "*tls_gd_pcrel<bits>" + [(set (match_operand:P 0 "gpc_reg_operand" "=b") + (unspec:P [(match_operand:P 1 "rs6000_tls_symbol_ref" "") + (const_int 0)] + UNSPEC_TLSGD))] + "HAVE_AS_TLS && TARGET_TLS_MARKERS" + "la %0,%1@got@tlsgd@pcrel" + [(set_attr "prefixed" "yes")]) + (define_insn_and_split "*tls_gd<bits>" [(set (match_operand:P 0 "gpc_reg_operand" "=b") (unspec:P [(match_operand:P 1 "rs6000_tls_symbol_ref" "") @@ -9554,6 +9564,14 @@ "HAVE_AS_TLS && TARGET_TLS_MARKERS && TARGET_CMODEL != CMODEL_SMALL" "addi %0,%1,%2@got@tlsgd@l") +(define_insn "*tls_ld_pcrel<bits>" + [(set (match_operand:P 0 "gpc_reg_operand" "=b") + (unspec:P [(const_int 0)] + UNSPEC_TLSLD))] + "HAVE_AS_TLS && TARGET_TLS_MARKERS" + "la %0,%&@got@tlsld@pcrel" + [(set_attr "prefixed" "yes")]) + (define_insn_and_split "*tls_ld<bits>" [(set (match_operand:P 0 "gpc_reg_operand" "=b") (unspec:P [(match_operand:P 1 "gpc_reg_operand" "b")] @@ -9597,7 +9615,11 @@ (match_operand:P 2 "rs6000_tls_symbol_ref" "")] UNSPEC_TLSDTPREL))] "HAVE_AS_TLS" - "addi %0,%1,%2@dtprel") + "addi %0,%1,%2@dtprel" + [(set (attr "prefixed") + (if_then_else (match_test "rs6000_tls_size == 16") + (const_string "no") + (const_string "yes")))]) (define_insn "tls_dtprel_ha_<bits>" [(set (match_operand:P 0 "gpc_reg_operand" "=r") @@ -9661,7 +9683,11 @@ (match_operand:P 2 "rs6000_tls_symbol_ref" "")] UNSPEC_TLSTPREL))] "HAVE_AS_TLS" - "addi %0,%1,%2@tprel") + "addi %0,%1,%2@tprel" + [(set (attr "prefixed") + (if_then_else (match_test "rs6000_tls_size == 16") + (const_string "no") + (const_string "yes")))]) (define_insn "tls_tprel_ha_<bits>" [(set (match_operand:P 0 "gpc_reg_operand" "=r") @@ -9679,6 +9705,15 @@ "HAVE_AS_TLS" "addi %0,%1,%2@tprel@l") +(define_insn "*tls_got_tprel_pcrel_<bits>" + [(set (match_operand:P 0 "gpc_reg_operand" "=b") + (unspec:P [(const_int 0) + (match_operand:P 1 "rs6000_tls_symbol_ref" "")] + UNSPEC_TLSGOTTPREL))] + "HAVE_AS_TLS" + "<ptrload> %0,%1@got@tprel@pcrel" + [(set_attr "prefixed" "yes")]) + ;; "b" output constraint here and on tls_tls input to support linker tls ;; optimization. The linker may edit the instructions emitted by a ;; tls_got_tprel/tls_tls pair to addis,addi. @@ -9722,6 +9757,14 @@ "HAVE_AS_TLS && TARGET_CMODEL != CMODEL_SMALL" "<ptrload> %0,%2@got@tprel@l(%1)") +(define_insn "tls_tls_pcrel_<bits>" + [(set (match_operand:P 0 "gpc_reg_operand" "=r") + (unspec:P [(match_operand:P 1 "gpc_reg_operand" "b") + (match_operand:P 2 "rs6000_tls_symbol_ref" "")] + UNSPEC_TLSTLS_PCREL))] + "TARGET_ELF && HAVE_AS_TLS" + "add %0,%1,%2@tls@pcrel") + (define_insn "tls_tls_<bits>" [(set (match_operand:P 0 "gpc_reg_operand" "=r") (unspec:P [(match_operand:P 1 "gpc_reg_operand" "b")
Supporting TLS for -mpcrel turns out to be relatively simple, in part due to deciding that !TARGET_TLS_MARKERS with -mpcrel is silly. No assembler that I know of supporting prefix insns lacks TLS marker support. Also, at some point powerpc gcc ought to remove !TARGET_TLS_MARKERS generally and simplify all the occurrences of IS_NOMARK_TLSGETADDR in rs6000.md rather than complicating them. Mike, the rs6000_option_override_internal hunk is new compared to the patch you had from me. * config/rs6000/predicates.md (unspec_tls): Allow const0_rtx for got element of unspec vec. * config/rs6000/rs6000.c (rs6000_option_override_internal): Disable -mpcrel if -mno-tls-markers. (rs6000_legitimize_tls_address): Support PC-relative TLS. * config/rs6000/rs6000.md (UNSPEC_TLSTLS_PCREL): New unspec. (tls_gd_pcrel, tls_ld_pcrel): New insns. (tls_dtprel, tls_tprel): Set attr prefixed when tls_size is not 16. (tls_got_tprel_pcrel, tls_tls_pcrel): New insns.