[PATCHv2/AARCH64,2/3] Fix TLS for ILP32.

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.  I modified them to be like what was done for
the GOT patterns.

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): Rename to ...
	(tlsie_small_<mode>): this and handle PTR.
	(tlsie_small_sidi): New pattern.
	(tlsle_small): Change to an expand to handle ILP32.
	(tlsle_small_<mode>): New pattern.
	(tlsdesc_small): Rename to ...
	(tlsdesc_small_<mode>): this and handle PTR.
---
 gcc/ChangeLog                 |   12 +++++++++
 gcc/config/aarch64/aarch64.c  |   48 +++++++++++++++++++++++++++++++----
 gcc/config/aarch64/aarch64.md |   54 +++++++++++++++++++++++++++++++---------
 3 files changed, 96 insertions(+), 18 deletions(-)

Hi,

On 02/26/14 02:25, 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.  I modified them to be like what was done for
> the GOT patterns.

The patch looks good to me (but I cannot approve).

There are some minor indentation issues though; see below.

>
> 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): Rename to ...
> 	(tlsie_small_<mode>): this and handle PTR.
> 	(tlsie_small_sidi): New pattern.
> 	(tlsle_small): Change to an expand to handle ILP32.
> 	(tlsle_small_<mode>): New pattern.
> 	(tlsdesc_small): Rename to ...
> 	(tlsdesc_small_<mode>): this and handle PTR.
> ---
>   gcc/ChangeLog                 |   12 +++++++++
>   gcc/config/aarch64/aarch64.c  |   48 +++++++++++++++++++++++++++++++----
>   gcc/config/aarch64/aarch64.md |   54 +++++++++++++++++++++++++++++++---------
>   3 files changed, 96 insertions(+), 18 deletions(-)

[snip]

> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 99a6ac8..7d8a645 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -3581,35 +3581,65 @@
>     [(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_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"
> +
> +(define_insn "tlsie_small_sidi"
>     [(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")]
> +	(zero_extend:DI
> +          (unspec:SI [(match_operand 1 "aarch64_tls_ie_symref" "S")]
> +		      UNSPEC_GOTSMALLTLS)))]
> +  ""
> +  "adrp\\t%0, %A1\;ldr\\t%w0, [%0, #%L1]"
> +  [(set_attr "type" "load1")
> +   (set_attr "length" "8")]
> +)
> +
> +
> +(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))]

The last two lines were not indented well.

> +  ""
> +{
> +  enum machine_mode mode = GET_MODE (operands[0]);
> +  emit_insn ((mode == DImode
> +	      ? gen_tlsle_small_di
> +	      : gen_tlsle_small_si) (operands[0],
> +				   operands[1],
> +				   operands[2]));

Likewise here.


Thanks,
Yufeng


> +  DONE;
> +
> +})
> +
> +(define_insn "tlsle_small_<mode>"
> +  [(set (match_operand:P 0 "register_operand" "=r")
> +        (unspec:P [(match_operand:P 1 "register_operand" "r")
> +                   (match_operand 2 "aarch64_tls_le_symref" "S")]
>   		   UNSPEC_GOTSMALLTLS))]
>     ""
> -  "add\\t%0, %1, #%G2\;add\\t%0, %0, #%L2"
> +  "add\\t%<w>0, %<w>1, #%G2\;add\\t%<w>0, %<w>0, #%L2"
>     [(set_attr "type" "alu_reg")
>      (set_attr "length" "8")]
>   )
>
> -(define_insn "tlsdesc_small"
> -  [(set (reg:DI R0_REGNUM)
> -        (unspec:DI [(match_operand:DI 0 "aarch64_valid_symref" "S")]
> +(define_insn "tlsdesc_small_<mode>"
> +  [(set (reg:PTR R0_REGNUM)
> +        (unspec:PTR [(match_operand 0 "aarch64_valid_symref" "S")]
>   		   UNSPEC_TLSDESC))
>      (clobber (reg:DI LR_REGNUM))
>      (clobber (match_scratch:DI 1 "=r"))]
>     "TARGET_TLS_DESC"
> -  "adrp\\tx0, %A0\;ldr\\t%1, [x0, #%L0]\;add\\tx0, x0, %L0\;.tlsdesccall\\t%0\;blr\\t%1"
> +  "adrp\\tx0, %A0\;ldr\\t%<w>1, [x0, #%L0]\;add\\t<w>0,<w>0, %L0\;.tlsdesccall\\t%0\;blr\\t%1"
>     [(set_attr "type" "call")
>      (set_attr "length" "16")])
>

On 26 February 2014 02:25, Andrew Pinski <apinski@cavium.com> 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.  I modified them to be like what was done for
> the GOT patterns.
>
> 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): Rename to ...
>         (tlsie_small_<mode>): this and handle PTR.
>         (tlsie_small_sidi): New pattern.
>         (tlsle_small): Change to an expand to handle ILP32.
>         (tlsle_small_<mode>): New pattern.
>         (tlsdesc_small): Rename to ...
>         (tlsdesc_small_<mode>): this and handle PTR.

Thanks Andrew, this is OK for stage-1, but please fix Yufeng's layout nit.
/Marcus

>      case SYMBOL_SMALL_GOTTPREL:
>        {
> -       rtx tmp_reg = gen_reg_rtx (Pmode);
> +       /* In ILP32, the mode of dest can be either SImode or DImode,
> +          while the got entry is always of SImode size.  The mode of
> +          dest depends on how dest is used: if dest is assigned to a
> +          pointer (e.g. in the memory), it has SImode; it may have
> +          DImode if dest is dereferenced to access the memeory.
> +          This is why we have to handle three different tlsie_small
> +          patterns here (two patterns for ILP32).  */
> +       enum machine_mode mode = GET_MODE (dest);
> +       rtx tmp_reg = gen_reg_rtx (mode);
>         rtx tp = aarch64_load_tp (NULL);
> -       emit_insn (gen_tlsie_small (tmp_reg, imm));
> -       emit_insn (gen_rtx_SET (Pmode, dest, gen_rtx_PLUS (Pmode, tp, tmp_reg)));
> +
> +       if (mode == ptr_mode)
> +         {
> +           if (mode == DImode)
> +             emit_insn (gen_tlsie_small_di (tmp_reg, imm));
> +           else
> +             {
> +               emit_insn (gen_tlsie_small_si (tmp_reg, imm));
> +               tp = gen_lowpart (mode, tp);
> +             }
> +         }
> +       else
> +         {
> +           gcc_assert (mode == Pmode);
> +           emit_insn (gen_tlsie_small_sidi (tmp_reg, imm));
> +         }


Andrew, do you recall the circumstance that trigger the mode == SImode
code path through the above code?  I've been trying to trigger this
while working on the equivalent tiny address mode code... with no
success.
/Marcus

On Fri, Feb 20, 2015 at 5:35 AM, Marcus Shawcroft
<marcus.shawcroft@gmail.com> wrote:
>>      case SYMBOL_SMALL_GOTTPREL:
>>        {
>> -       rtx tmp_reg = gen_reg_rtx (Pmode);
>> +       /* In ILP32, the mode of dest can be either SImode or DImode,
>> +          while the got entry is always of SImode size.  The mode of
>> +          dest depends on how dest is used: if dest is assigned to a
>> +          pointer (e.g. in the memory), it has SImode; it may have
>> +          DImode if dest is dereferenced to access the memeory.
>> +          This is why we have to handle three different tlsie_small
>> +          patterns here (two patterns for ILP32).  */
>> +       enum machine_mode mode = GET_MODE (dest);
>> +       rtx tmp_reg = gen_reg_rtx (mode);
>>         rtx tp = aarch64_load_tp (NULL);
>> -       emit_insn (gen_tlsie_small (tmp_reg, imm));
>> -       emit_insn (gen_rtx_SET (Pmode, dest, gen_rtx_PLUS (Pmode, tp, tmp_reg)));
>> +
>> +       if (mode == ptr_mode)
>> +         {
>> +           if (mode == DImode)
>> +             emit_insn (gen_tlsie_small_di (tmp_reg, imm));
>> +           else
>> +             {
>> +               emit_insn (gen_tlsie_small_si (tmp_reg, imm));
>> +               tp = gen_lowpart (mode, tp);
>> +             }
>> +         }
>> +       else
>> +         {
>> +           gcc_assert (mode == Pmode);
>> +           emit_insn (gen_tlsie_small_sidi (tmp_reg, imm));
>> +         }
>
>
> Andrew, do you recall the circumstance that trigger the mode == SImode
> code path through the above code?  I've been trying to trigger this
> while working on the equivalent tiny address mode code... with no
> success.

IIRC this happened during the compiling of glibc.  That is with -fPIC
and with initial-exec tls_model.  errno variable in fact.

Thanks,
Andrew Pinski



> /Marcus