Re: [PATCH/AARCH64 5/6] Fix TLS for ILP32.

[Yufeng Zhang <Yufeng dot Zhang at arm dot com>]

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.

Same for tlsie_small above.

> +  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.


Yufeng