Re: [Patch, updated] Make emulated TLS lto-friendly.

[IainS <developer at sandoe-acoustics dot co dot uk>]

On 8 Jul 2010, at 20:19, Richard Henderson wrote:

> On 07/08/2010 12:07 PM, IainS wrote:
> > On 8 Jul 2010, at 00:22, Richard Henderson wrote:
> > > Do you really need this kind of thing anymore, since you're exposing
> > > the use of the control variable so early?  I would have thought that
> > > varpool.c would no longer need any special-casing for !have_tls.
> >
> > this is my understanding (which might be flawed &| incomplete).
> >
> > Whilst we are on the parse side - and building varpool & cgraph, the
> > relationship is not fully exposed (that happens when gimplication  
> > is done).
> >
> > So my reasoning was that the "ghost/proxy" vars should be made to  
> > track
> > the user-land ones until then.
>
> Hmm.  So what you're saying is that there's extra cleanup work that
> needs to happen while lowering the representation.  It's not merely
> a matter of code substitution during gimplification.

Attached is an updated patch which addresses the points you raised -  
less my response above.

IMO it is not the world's most elegant solution..
... perhaps in part owing to me not know the "Best Places" to do some  
things
... but I suspect also because the whole thing doesn't sit comfortably..

I'm not sure where we sit in terms of applying this..
I guess it remains an interim solution to LTO on emuTLS targets, with  
the observation that perhaps a different approach is needed now we  
have LTO.

I've tested on i686-apple-darwin9 and cris-elf that the changes don't  
regress.
(in fact, my local tls testsuite is somewhat wider than the one in  
trunk)

If it is intended to apply this then, of course, I'd test on linux  
first (should be a nil response).

Known Minus:
 it fails the regression Jakub pointed out.

Known Plus:
  it allows lto
 it actually performs better on CSE than trunk does (test added).

I'd hazard that the CSE improvement is worth more than the lost  
trivial zero return.

> In which case I wonder if it wouldn't be better to do as Honza
> suggested and separate all of this out into a new pass_lower_emutls.
> Perhaps to be placed just after pass_lower_vector.  That placement
> is before pass_build_cgraph_edges, which I believe means you would
> not have to fix up the cgraph edges for the new function calls.  All
> you'd need to do is transform the tls variable references and fix up
> the varpool references.

My £0.02 having tangled with this for the last few weeks...

1/ a pass behind a gate of ! targetm.have_tls is a whole lot less  
intrusive to non-emutls targets than what we have
2/ it's likely more efficient for emutls targets since there's less to- 
ing and fro-ing.
3/ probably more maintainable.
4/ certainly more transparent.
5/ (probably) loses all reference to emutls from varasm, varpool,  
expr, & gimplify..

mind you, that doesn't mean I know how to write such a pass ...  ;-)

Iain.

gcc:

	PR target/44132
	* expr.c (emutls_var_address): Remove.
	(expand_expr_addr_expr_1): Remove TLS emulation hook.
	(expand_expr_real_1): Ditto.

	* gimplify.c (emutls_var_address): Add proc.
	(gimplify_decl_expr): expand TLS vars.
	(gimplify_var_or_parm_decl): Ditto.
	(omp_notice_variable): Recognize TLS_MODEL_EMULATED.

	* passes.c (rest_of_decl_compilation): Substitute TLS control vars  
for the master.

	* varasm.c (decl_needs_tls_emulation_p): New.
	(get_emutls_init_templ_addr): Adjust DECL_PRESERVE_P and DECL_INITIAL.
	(emutls_decl): Copy TREE_ADDRESSABLE, DECL_PRESERVE_P, create an
	initializer when one is present on the user-var.
	(emutls_common_1): Remove comment.
	(emutls_finalize_control_var): Copy TREE_USED, add an initializer for  
size and
	align fields for cases with un-initialized user vars.
	(emutls_find_user_var_cb): New.
	(emutls_add_base_initializer): New.
	(asm_output_bss): Assert not a tls var needing emulation.
	(asm_output_aligned_bss): Ditto.
	(assemble_variable): Remove control var init. code.  Assert not a tls  
var needing
	emulation.  Provide a trivial initializer for size and align fields  
if one is not already
	set. (do_assemble_alias): Do not handle emutls vars here.
	(var_decl_for_asm): New.
	(finish_aliases_1): Walk the alias pairs substituting emutls controls  
for the user
	counterparts.

	* varpool.c (varpool_mark_needed_node): Do not handle TLS  
substitution here.
	(decide_is_variable_needed): Or here.
	(varpool_finalize_decl): Handle TLS substitution.
	Remove early calls to varpool_assemble_pending_decls().
	Check enqueuing of vars after all tests for need are complete.
	(varpool_analyze_pending_decls): Do not record references if the  
initializer is error_mark.

testsuite:
	
	PR target/44132
	* gcc.dg/tls/thr-init-1.c: New.
	* gcc.dg/tls/thr-init-2.c: New.
	* gcc.dg/torture/tls New.
	* gcc.dg/torture/tls/tls-test.c: New.
	* gcc.dg/torture/tls/thr-init-1.c: New.
	* gcc.dg/torture/tls/tls.exp: New.
	* gcc.dg/torture/tls/thr-init-2.c: New.

	* lib/target-supports.exp (check_effective_target_tls_emulated): New.
	* gcc.dg/tls/thr-cse-1.c: New.

Attachment: 161974-emutls-lto.txt
Description: Text document

Attachment: 161974-emutls-lto-testsuite.txt
Description: Text document