* [PATCH] Fix target/93119 (aarch64): ICE with traditional TLS support on ILP32
@ 2020-01-18 0:08 apinski
2020-01-20 12:16 ` Richard Sandiford
0 siblings, 1 reply; 3+ messages in thread
From: apinski @ 2020-01-18 0:08 UTC (permalink / raw)
To: gcc-patches; +Cc: Andrew Pinski
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
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;
+}
--
2.17.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix target/93119 (aarch64): ICE with traditional TLS support on ILP32
2020-01-18 0:08 [PATCH] Fix target/93119 (aarch64): ICE with traditional TLS support on ILP32 apinski
@ 2020-01-20 12:16 ` Richard Sandiford
2020-01-22 1:58 ` apinski
0 siblings, 1 reply; 3+ messages in thread
From: Richard Sandiford @ 2020-01-20 12:16 UTC (permalink / raw)
To: apinski; +Cc: gcc-patches
<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;
> +}
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] Fix target/93119 (aarch64): ICE with traditional TLS support on ILP32
2020-01-20 12:16 ` Richard Sandiford
@ 2020-01-22 1:58 ` apinski
0 siblings, 0 replies; 3+ messages in thread
From: apinski @ 2020-01-22 1:58 UTC (permalink / raw)
To: richard.sandiford, gcc-patches; +Cc: Andrew Pinski
From: Andrew Pinski <apinski@marvell.com>
This is what I committed after Richard's comments.
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.
ChangeLog:
PR target/93119
* 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.
---
gcc/ChangeLog | 11 +++++++++++
gcc/config/aarch64/aarch64.c | 17 +++++++++++++----
gcc/config/aarch64/aarch64.md | 8 ++++----
gcc/testsuite/gcc.target/aarch64/pr93119.c | 10 ++++++++++
4 files changed, 38 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr93119.c
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 2f6e603df7a..85cf788cc50 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,14 @@
+2020-01-21 Andrew Pinski <apinski@marvell.com>
+
+ PR target/9311
+ * 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.
+
+
2020-01-21 Jim Wilson <jimw@sifive.com>
* config/riscv/riscv-sr.c (riscv_sr_match_prologue): Use INTVAL
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index ef037e226a7..9acf33dbe64 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2607,11 +2607,16 @@ 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));
@@ -2619,7 +2624,11 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm,
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
+ from SImode (ptr_mode) to DImode (Pmode). */
+ 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;
+}
--
2.17.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-01-22 1:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-18 0:08 [PATCH] Fix target/93119 (aarch64): ICE with traditional TLS support on ILP32 apinski
2020-01-20 12:16 ` Richard Sandiford
2020-01-22 1:58 ` apinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).