On Wed, Dec 4, 2013 at 10:12 AM, Yufeng Zhang <yufeng.zh...@arm.com> wrote:
> On 12/03/13 21:24, Andrew Pinski wrote:
>>
>> Hi,
>> With ILP32, some simple usage of TLS variables causes an unrecognizable
>> instruction due to needing to use SImode for loading pointers from memory.
>> This fixes the three (tlsie_small, tlsle_small, tlsdesc_small) patterns to
>> support SImode for pointers.
>>
>> OK? Build and tested on aarch64-elf with no regressions.
>>
>> Thanks,
>> Andrew Pinski
>>
>> * config/aarch64/aarch64.c (aarch64_load_symref_appropriately):
>> Handle TLS for ILP32.
>> * config/aarch64/aarch64.md (tlsie_small): Change to an expand to
>> handle ILP32.
>> (tlsie_small_<mode>): New pattern.
>> (tlsle_small): Change to an expand to handle ILP32.
>> (tlsle_small_<mode>): New pattern.
>> (tlsdesc_small): Change to an expand to handle ILP32.
>> (tlsdesc_small_<mode>): New pattern.
>> ---
>> gcc/ChangeLog | 12 ++++++
>> gcc/config/aarch64/aarch64.c | 23 ++++++++++--
>> gcc/config/aarch64/aarch64.md | 76
>> ++++++++++++++++++++++++++++++++++-------
>> 3 files changed, 94 insertions(+), 17 deletions(-)
>>
>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
>> index b1b4eef..a3e4532 100644
>> --- a/gcc/config/aarch64/aarch64.c
>> +++ b/gcc/config/aarch64/aarch64.c
>> @@ -628,22 +628,37 @@ aarch64_load_symref_appropriately (rtx dest, rtx
>> imm,
>>
>> case SYMBOL_SMALL_TLSDESC:
>> {
>> - rtx x0 = gen_rtx_REG (Pmode, R0_REGNUM);
>> + enum machine_mode mode = GET_MODE (dest);
>> + rtx x0 = gen_rtx_REG (mode, R0_REGNUM);
>> rtx tp;
>>
>> + gcc_assert (mode == Pmode || mode == ptr_mode);
>> +
>> emit_insn (gen_tlsdesc_small (imm));
>> tp = aarch64_load_tp (NULL);
>> - emit_insn (gen_rtx_SET (Pmode, dest, gen_rtx_PLUS (Pmode, tp,
>> x0)));
>> +
>> + if (mode != Pmode)
>> + tp = gen_lowpart (mode, tp);
>> +
>> + emit_insn (gen_rtx_SET (mode, dest, gen_rtx_PLUS (mode, tp, x0)));
>> set_unique_reg_note (get_last_insn (), REG_EQUIV, imm);
>> return;
>> }
>>
>> case SYMBOL_SMALL_GOTTPREL:
>> {
>> - rtx tmp_reg = gen_reg_rtx (Pmode);
>> + enum machine_mode mode = GET_MODE (dest);
>> + rtx tmp_reg = gen_reg_rtx (mode);
>> rtx tp = aarch64_load_tp (NULL);
>> +
>> + gcc_assert (mode == Pmode || mode == ptr_mode);
>> +
>> emit_insn (gen_tlsie_small (tmp_reg, imm));
>> - emit_insn (gen_rtx_SET (Pmode, dest, gen_rtx_PLUS (Pmode, tp,
>> tmp_reg)));
>> +
>> + if (mode != Pmode)
>> + tp = gen_lowpart (mode, tp);
>> +
>> + emit_insn (gen_rtx_SET (mode, dest, gen_rtx_PLUS (mode, tp,
>> tmp_reg)));
>> set_unique_reg_note (get_last_insn (), REG_EQUIV, imm);
>> return;
>> }
>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>> index 313517f..08fcc94 100644
>> --- a/gcc/config/aarch64/aarch64.md
>> +++ b/gcc/config/aarch64/aarch64.md
>> @@ -3577,35 +3577,85 @@
>> [(set_attr "type" "call")
>> (set_attr "length" "16")])
>>
>> -(define_insn "tlsie_small"
>> - [(set (match_operand:DI 0 "register_operand" "=r")
>> - (unspec:DI [(match_operand:DI 1 "aarch64_tls_ie_symref" "S")]
>> +(define_expand "tlsie_small"
>> + [(set (match_operand 0 "register_operand" "=r")
>> + (unspec [(match_operand 1 "aarch64_tls_ie_symref" "S")]
>> + UNSPEC_GOTSMALLTLS))]
>> + ""
>> +{
>> + if (TARGET_ILP32)
>> + {
>> + operands[0] = gen_lowpart (ptr_mode, operands[0]);
>> + emit_insn (gen_tlsie_small_si (operands[0], operands[1]));
>> + }
>> + else
>> + emit_insn (gen_tlsie_small_di (operands[0], operands[1]));
>> + DONE;
>> +})
>> +
>> +(define_insn "tlsie_small_<mode>"
>> + [(set (match_operand:PTR 0 "register_operand" "=r")
>> + (unspec:PTR [(match_operand 1 "aarch64_tls_ie_symref" "S")]
>> UNSPEC_GOTSMALLTLS))]
>> ""
>> - "adrp\\t%0, %A1\;ldr\\t%0, [%0, #%L1]"
>> + "adrp\\t%0, %A1\;ldr\\t%<w>0, [%0, #%L1]"
>> [(set_attr "type" "load1")
>> (set_attr "length" "8")]
>> )
>>
>> -(define_insn "tlsle_small"
>> - [(set (match_operand:DI 0 "register_operand" "=r")
>> - (unspec:DI [(match_operand:DI 1 "register_operand" "r")
>> - (match_operand:DI 2 "aarch64_tls_le_symref" "S")]
>> +
>> +(define_expand "tlsle_small"
>> + [(set (match_operand 0 "register_operand" "=r")
>> + (unspec [(match_operand 1 "register_operand" "r")
>> + (match_operand 2 "aarch64_tls_le_symref" "S")]
>> + UNSPEC_GOTSMALLTLS))]
>> + ""
>> +{
>> + if (TARGET_ILP32)
>> + {
>> + rtx temp = gen_reg_rtx (ptr_mode);
>> + operands[1] = gen_lowpart (ptr_mode, operands[1]);
>> + emit_insn (gen_tlsle_small_si (temp, operands[1], operands[2]));
>> + emit_move_insn (operands[0], gen_lowpart (GET_MODE (operands[0]),
>> temp));
>> + }
>
>
> Looks like you hit the similar issue where the matched RTX can have either
> SImode or DImode in ILP32. The mechanism looks OK but I think the approach
> that 'add_losym' adopts is neater, which checks on the mode instead of
> TARGET_ILP32 and calls gen_add_losym_di or gen_add_losym_si accordingly.
> Note that the iterator used in add_losym_<mode> is P instead of PTR.
Yes I agree with this and will fix this.
>
> Same for tlsie_small above.
But not with this one, tlsie_small should rather be similar to
ldr_got_small instead.
>
>
>> + else
>> + emit_insn (gen_tlsle_small_di (operands[0], operands[1],
>> operands[2]));
>> + DONE;
>
>
> Brackets for the else body to align with the if body; same for tlsie_small
> above.
There is no requirement in the coding style for this. In fact there
are examples of the coding style I used here all over GCC; even in
aarch64.md (rotl<mode>3) and aarch64.c
(aarch64_function_arg_alignment).
>
>
> Yufeng
>
