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

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

On 02/25/14 01:23, Andrew Pinski wrote:
> On Wed, Dec 4, 2013 at 10:12 AM, Yufeng Zhang<Yufeng.Zhang@arm.com>  wrote:
> > On 12/03/13 21:24, Andrew Pinski wrote:

[snip]

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

Agree; tlsie_small needs to handle the load of an SImode-sized item from 
an address having DImode.


Yufeng