Message ID | 1579302817-19505-1-git-send-email-apinski@marvell.com |
---|---|
State | New |
Headers | show |
Series | Fix target/93119 (aarch64): ICE with traditional TLS support on ILP32 | expand |
<apinski@marvell.com> writes: > From: Andrew Pinski <apinski@marvell.com> > > The problem here was g:23b88fda665d2f995c was not a complete fix > for supporting tranditional TLS on ILP32. > > So the problem here is a couple of things, first __tls_get_addr > call will return a C pointer value so we need to use ptr_mode > when we are creating the call. Then we need to convert > back that register to the correct mode, either zero extending > it or just creating a move instruction. This and the comment: > + /* Convert back to the mode of the dest, adding a zero-extend as > + needed. */ > + if (dest != tmp_reg) > + convert_move (dest, tmp_reg, true); make it sound like the convert_move is handling two separate cases. But AFAICT the convert_move only ever zero-extends, it should never emit just a plain move. Is that right? > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index c26ac0db942..6c825459dc6 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -2607,19 +2607,29 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm, > case SYMBOL_SMALL_TLSGD: > { > rtx_insn *insns; > - machine_mode mode = GET_MODE (dest); > - rtx result = gen_rtx_REG (mode, R0_REGNUM); > + /* The return type of __tls_get_addr is the C pointer type > + so use ptr_mode. */ > + rtx result = gen_rtx_REG (ptr_mode, R0_REGNUM); > + rtx tmp_reg = dest; > + > + if (GET_MODE (dest) != ptr_mode) > + tmp_reg = can_create_pseudo_p () ? gen_reg_rtx (ptr_mode) : result; > > start_sequence (); > - if (TARGET_ILP32) > + if (ptr_mode == SImode) > aarch64_emit_call_insn (gen_tlsgd_small_si (result, imm)); > else > aarch64_emit_call_insn (gen_tlsgd_small_di (result, imm)); > + > insns = get_insns (); > end_sequence (); I think this looks more obvious without the extra blank line, so that the whole start_sequence/end_sequence is a single block. If the convert_move does always zero-extend, the patch is OK without the extra blank line and with an updated comment. Thanks, Richard > > RTL_CONST_CALL_P (insns) = 1; > - emit_libcall_block (insns, dest, result, imm); > + emit_libcall_block (insns, tmp_reg, result, imm); > + /* Convert back to the mode of the dest, adding a zero-extend as > + needed. */ > + if (dest != tmp_reg) > + convert_move (dest, tmp_reg, true); > return; > } > > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md > index 86c2cdfc797..55dde54b16a 100644 > --- a/gcc/config/aarch64/aarch64.md > +++ b/gcc/config/aarch64/aarch64.md > @@ -6755,10 +6755,10 @@ (define_insn "aarch64_load_tp_hard" > ;; instructions in the TLS stubs, in order to enable linker relaxation. > ;; Therefore we treat the stubs as an atomic sequence. > (define_expand "tlsgd_small_<mode>" > - [(parallel [(set (match_operand 0 "register_operand") > + [(parallel [(set (match_operand:PTR 0 "register_operand") > (call (mem:DI (match_dup 2)) (const_int 1))) > (unspec:DI [(const_int 0)] UNSPEC_CALLEE_ABI) > - (unspec:DI [(match_operand:PTR 1 "aarch64_valid_symref")] UNSPEC_GOTSMALLTLS) > + (unspec:DI [(match_operand 1 "aarch64_valid_symref")] UNSPEC_GOTSMALLTLS) > (clobber (reg:DI LR_REGNUM))])] > "" > { > @@ -6766,10 +6766,10 @@ (define_expand "tlsgd_small_<mode>" > }) > > (define_insn "*tlsgd_small_<mode>" > - [(set (match_operand 0 "register_operand" "") > + [(set (match_operand:PTR 0 "register_operand" "") > (call (mem:DI (match_operand:DI 2 "" "")) (const_int 1))) > (unspec:DI [(const_int 0)] UNSPEC_CALLEE_ABI) > - (unspec:DI [(match_operand:PTR 1 "aarch64_valid_symref" "S")] UNSPEC_GOTSMALLTLS) > + (unspec:DI [(match_operand 1 "aarch64_valid_symref" "S")] UNSPEC_GOTSMALLTLS) > (clobber (reg:DI LR_REGNUM)) > ] > "" > diff --git a/gcc/testsuite/gcc.target/aarch64/pr93119.c b/gcc/testsuite/gcc.target/aarch64/pr93119.c > new file mode 100644 > index 00000000000..93fa80e10b6 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/pr93119.c > @@ -0,0 +1,10 @@ > +/* { dg-require-effective-target fpic } */ > +/* { dg-options "-mtls-dialect=trad -fpic" } */ > + > +__thread int g_tlsdata; > + > +int func1() > +{ > + g_tlsdata++; > + return g_tlsdata; > +}
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index c26ac0db942..6c825459dc6 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -2607,19 +2607,29 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm, case SYMBOL_SMALL_TLSGD: { rtx_insn *insns; - machine_mode mode = GET_MODE (dest); - rtx result = gen_rtx_REG (mode, R0_REGNUM); + /* The return type of __tls_get_addr is the C pointer type + so use ptr_mode. */ + rtx result = gen_rtx_REG (ptr_mode, R0_REGNUM); + rtx tmp_reg = dest; + + if (GET_MODE (dest) != ptr_mode) + tmp_reg = can_create_pseudo_p () ? gen_reg_rtx (ptr_mode) : result; start_sequence (); - if (TARGET_ILP32) + if (ptr_mode == SImode) aarch64_emit_call_insn (gen_tlsgd_small_si (result, imm)); else aarch64_emit_call_insn (gen_tlsgd_small_di (result, imm)); + insns = get_insns (); end_sequence (); RTL_CONST_CALL_P (insns) = 1; - emit_libcall_block (insns, dest, result, imm); + emit_libcall_block (insns, tmp_reg, result, imm); + /* Convert back to the mode of the dest, adding a zero-extend as + needed. */ + if (dest != tmp_reg) + convert_move (dest, tmp_reg, true); return; } diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 86c2cdfc797..55dde54b16a 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -6755,10 +6755,10 @@ (define_insn "aarch64_load_tp_hard" ;; instructions in the TLS stubs, in order to enable linker relaxation. ;; Therefore we treat the stubs as an atomic sequence. (define_expand "tlsgd_small_<mode>" - [(parallel [(set (match_operand 0 "register_operand") + [(parallel [(set (match_operand:PTR 0 "register_operand") (call (mem:DI (match_dup 2)) (const_int 1))) (unspec:DI [(const_int 0)] UNSPEC_CALLEE_ABI) - (unspec:DI [(match_operand:PTR 1 "aarch64_valid_symref")] UNSPEC_GOTSMALLTLS) + (unspec:DI [(match_operand 1 "aarch64_valid_symref")] UNSPEC_GOTSMALLTLS) (clobber (reg:DI LR_REGNUM))])] "" { @@ -6766,10 +6766,10 @@ (define_expand "tlsgd_small_<mode>" }) (define_insn "*tlsgd_small_<mode>" - [(set (match_operand 0 "register_operand" "") + [(set (match_operand:PTR 0 "register_operand" "") (call (mem:DI (match_operand:DI 2 "" "")) (const_int 1))) (unspec:DI [(const_int 0)] UNSPEC_CALLEE_ABI) - (unspec:DI [(match_operand:PTR 1 "aarch64_valid_symref" "S")] UNSPEC_GOTSMALLTLS) + (unspec:DI [(match_operand 1 "aarch64_valid_symref" "S")] UNSPEC_GOTSMALLTLS) (clobber (reg:DI LR_REGNUM)) ] "" diff --git a/gcc/testsuite/gcc.target/aarch64/pr93119.c b/gcc/testsuite/gcc.target/aarch64/pr93119.c new file mode 100644 index 00000000000..93fa80e10b6 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/pr93119.c @@ -0,0 +1,10 @@ +/* { dg-require-effective-target fpic } */ +/* { dg-options "-mtls-dialect=trad -fpic" } */ + +__thread int g_tlsdata; + +int func1() +{ + g_tlsdata++; + return g_tlsdata; +}
From: Andrew Pinski <apinski@marvell.com> The problem here was g:23b88fda665d2f995c was not a complete fix for supporting tranditional TLS on ILP32. So the problem here is a couple of things, first __tls_get_addr call will return a C pointer value so we need to use ptr_mode when we are creating the call. Then we need to convert back that register to the correct mode, either zero extending it or just creating a move instruction. Also symbol_ref can either be in SImode or DImode. So we need to allow both modes. Built and tested on aarch64-linux-gnu with no regressions. Also built a full toolchain (including glibc) defaulting to traditional TLS that targets ilp32 and lp64. OK? Thanks, Andrew Pinski ChangeLog: * config/aarch64/aarch64.md (tlsgd_small_<mode>): Have operand 0 as PTR mode. Have operand 1 as being modeless, it can be P mode. (*tlsgd_small_<mode>): Likewise. * config/aarch64/aarch64.c (aarch64_load_symref_appropriately) <case SYMBOL_SMALL_TLSGD>: Call gen_tlsgd_small_* with a ptr_mode register. Convert that register back to dest using convert_mode. Change-Id: I76826350d6bace0b731f21df1f125e9122da843f --- gcc/config/aarch64/aarch64.c | 18 ++++++++++++++---- gcc/config/aarch64/aarch64.md | 8 ++++---- gcc/testsuite/gcc.target/aarch64/pr93119.c | 10 ++++++++++ 3 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/gcc.target/aarch64/pr93119.c