Re: [PATCH/RFA] SH TLS support

[Joern Rennecke <joern dot rennecke at superh dot com>]

kaz Kojima wrote:
> 
> Hi,
> 
> This adds the TLS support for sh[34]-unknown-linux-gnu targets.
> I've used it for glibc and nptl work for a while and could find no
> obvious flaws.
> It bootstrapped on i686-pc-linux-gnu and sh4-unknown-linux-gnu without
> no new regressions. All TLS tests pass on sh4-unknown-linux-gnu. It
> was also regtested on cross sh64-unknown-linux-gnu.
> Ok for mainline?
> 
> Regards,
>         kaz

> +;; TLS code generation.
> +
> +(define_insn "tls_global_dynamic"
> +  [(set (match_operand:SI 0 "register_operand" "=&z")
> +       (unspec:SI [(match_operand:SI 1 "" "")]
> +                   UNSPEC_TLSGD))
> +   (use (reg:PSI FPSCR_REG))
> +   (use (reg:SI PIC_REG))
> +   (clobber (reg:SI PR_REG))
> +   (clobber (scratch:SI))]
> +  "TARGET_SH1"
> +  "*
> +{
> +  return \"\\
> +mov.l\\t1f,r4\\n\\
> +\\tmova\\t2f,r0\\n\\
> +\\tmov.l\\t2f,r1\\n\\
> +\\tadd\\tr0,r1\\n\\
> +\\tjsr\\t@r1\\n\\
> +\\tadd\\tr12,r4\\n\\
> +\\tbra\\t3f\\n\\
> +\\tnop\\n\\
> +\\t.align\\t2\\n\\
> +1:\\t.long\\t%a1@TLSGD\\n\\
> +2:\\t.long\\t__tls_get_addr@PLT\\n\\
> +3:\";
> +}"
> +  [(set_attr "type" "tls_load")
> +   (set_attr "length" "26")])

I'd like to see us move away from putting random tiny blobs of data into
the text, rather than adding more.  Putting a few data bytes here and there
pollutes both data and instruction cache.
I think you should be able to represent the constants as V2SI data, which
can stay there during reload, and then should be put into a constant pool
by machine_dependent_reorg.  Or if the two values may be placed independently,
you can just have them as two SImode values.
Moreover, do you actually need the value in r0 for anything but calculating
the jsr destination?  SH2 and later have the bsr instruction, so if we can
just add the appropriate negative offset to the value at 1f, we get a much
shorter sequence:

	mov.l L2,r1
	mov.l L1,r4
	bsr @r1
	add r12,r4
L0:
....
(in constant pool)
L1:	.long	%a1@TLSGD	
L2:	.long	__tls_get_addr@PLT+(L2-L0)

This should actually be a define_insn_and_split, with the splitting
happening after reload (but before sched2 & machine_dependent_reorg),
to each of the load separately.  The bsr, the add and the label shown
as L0 here best stay together as one nominal instruction, so that we
don't get any problems with instruction movement & duplication.
The label can be implicit in an unspec value we use for the
__tls_get_addr@PLT+(L2-L0) expression, and machine_dependent_reorg
will have to make it explicit when it creates the constant pool entry,
or - simpler, but more fragile, but that should really not be disturbed
after machine_dependent_reorg - represent that label by a reference to the
bsr/add/label instruction.
	
-- 
--------------------------
SuperH (UK) Ltd.
2410 Aztec West / Almondsbury / BRISTOL / BS32 4QX
T:+44 1454 465658