[trans-mem] undefined reference to a static cloned function

> gcc -fgnu-tm -Wall -DNDEBUG -O3 element.c -o element -litm -lm
> /tmp/ccEhZPec.o:(.tm_clone_table+0x18): undefined reference to
> `transaction clone for calculateCircumCircle'

In the testcase below we have a clone that gets inlined and optimized
away, so it doesn't get outputted, but we still dump its address into
the clone table.

I've fixed this by not dumping entries into the clone table for which
clones are not needed.  Previously we were looking at the neediness of
the original function, but since we now mark clones as needed in
ipa_tm_insert_gettmclone_call(), we are assured to have the needy bit
set correctly.

Also, I got tired of IPA dumps not distinguishing between regular
functions and TM clones.  I've made the CFG dumper distinguish between
the two.

OK for branch?

	* varasm.c (finish_tm_clone_pairs_1): 
	* tree-cfg.c (dump_function_to_file): Display [tm-clone] if
	applicable.

On 06/10/2010 06:37 AM, Aldy Hernandez wrote:
> 	* varasm.c (finish_tm_clone_pairs_1): 
> 	* tree-cfg.c (dump_function_to_file): Display [tm-clone] if
> 	applicable.

Ok if you finish writing the changelog entry for finish_tm_clone_pairs_1.


r~

Hello Aldy,

> +element_t*
> +element_alloc (coordinate_t* coordinates, long numCoordinate)
> +{
> +    element_t* elementPtr;
> +    elementPtr = (element_t*)xmalloc(sizeof(element_t));
> +    if (elementPtr) {
> +        calculateCircumCircle(elementPtr);
> +    }
> +    return elementPtr;
> +}
> +
> +__attribute__((transaction_safe))
> +element_t*
> +TMelement_alloc (coordinate_t* coordinates, long numCoordinate)
> +{
> +    element_t* elementPtr;
> +    elementPtr = (element_t*)xmalloc(sizeof(element_t));
> +    if (elementPtr) {
> +        calculateCircumCircle(elementPtr);
> +    }
> +    return elementPtr;
> +}

As we can see here, calculateCircumCircle function can be called into a 
transaction_safe function but also in a regular function. So the 
function calculateCircumCircle must have the transaction_safe attribute 
but...

if calculateCircumCircle is declared and defined like that (without 
transaction_safe):
static void
calculateCircumCircle (element_t* elementPtr)

GCC doesn't complain about transaction_safe attribute. The reason could 
be that the elementPtr is allocated inside the transaction (is gcc-tm 
able to detect that?) but if I modify the function TMelement_alloc to 
use a global variable, gcc still doesn't complain. So I don't understand 
the behaviour here.

Can you just explain me why it doesn't complain that the 
calculateCircumCircle isn't safe? is it a mistake?

Thank you,

Patrick.

> As we can see here, calculateCircumCircle function can be called into a  
> transaction_safe function but also in a regular function. So the  
> function calculateCircumCircle must have the transaction_safe attribute  
> but...
>
> if calculateCircumCircle is declared and defined like that (without  
> transaction_safe):
> static void
> calculateCircumCircle (element_t* elementPtr)
>
> GCC doesn't complain about transaction_safe attribute. The reason could  

GCC doesn't complain because it can determine that calculateCircumCircle
never enters serial irrevocable mode (besides I believe in this case the
compiler inlines the call to calculateCircumCircle).

This exclusion is done in ipa_tm_diagnose_tm_safe():

	 if (!is_tm_callable (e->callee->decl)
		&& e->callee->local.tm_may_enter_irr)
	      error_at (gimple_location (e->call_stmt),

I suppose we could warn regardless.  I don't know if having the compiler
outsmart us was part of rth's plan.  Richard?

Aldy

On 06/15/2010 07:41 AM, Aldy Hernandez wrote:
>> As we can see here, calculateCircumCircle function can be called into a  
>> transaction_safe function but also in a regular function. So the  
>> function calculateCircumCircle must have the transaction_safe attribute  
>> but...
>>
>> if calculateCircumCircle is declared and defined like that (without  
>> transaction_safe):
>> static void
>> calculateCircumCircle (element_t* elementPtr)
>>
>> GCC doesn't complain about transaction_safe attribute. The reason could  
> 
> GCC doesn't complain because it can determine that calculateCircumCircle
> never enters serial irrevocable mode (besides I believe in this case the
> compiler inlines the call to calculateCircumCircle).
> 
> This exclusion is done in ipa_tm_diagnose_tm_safe():
> 
> 	 if (!is_tm_callable (e->callee->decl)
> 		&& e->callee->local.tm_may_enter_irr)
> 	      error_at (gimple_location (e->call_stmt),
> 
> I suppose we could warn regardless.  I don't know if having the compiler
> outsmart us was part of rth's plan.  Richard?

The intent of the spec is that static functions need not be explicitly
annotated, since the compiler *can* derive all the information it needs,
including implicitly generating a clone.

I've sort of lost the thread here... does that even answer the question?


r~

On 06/15/2010 06:17 PM, Richard Henderson wrote:
> On 06/15/2010 07:41 AM, Aldy Hernandez wrote:
>>> As we can see here, calculateCircumCircle function can be called into a
>>> transaction_safe function but also in a regular function. So the
>>> function calculateCircumCircle must have the transaction_safe attribute
>>> but...
>>>
>>> if calculateCircumCircle is declared and defined like that (without
>>> transaction_safe):
>>> static void
>>> calculateCircumCircle (element_t* elementPtr)
>>>
>>> GCC doesn't complain about transaction_safe attribute. The reason could
>>
>> GCC doesn't complain because it can determine that calculateCircumCircle
>> never enters serial irrevocable mode (besides I believe in this case the
>> compiler inlines the call to calculateCircumCircle).
>
> The intent of the spec is that static functions need not be explicitly
> annotated, since the compiler *can* derive all the information it needs,
> including implicitly generating a clone.
>
> I've sort of lost the thread here... does that even answer the question?

Yes! Thank you very much :)
Since most functions need transaction_safe, I was supposing it was 
mandatory. Arg sorry for the noise.

Patrick