[v3] gcc/config/tilegx/tilegx.c (tilegx_function_profiler): Save r10 to stack before call mcount

From: Chen Gang <gang.chen.5i5j@gmail.com>

r10 may also be as parameter stack pointer for the nested function, so
need save it before call mcount.

Also clean up code: use '!' instead of "== 0" for checking
static_chain_decl and compute_total_frame_size.

2016-06-04  Chen Gang  <gang.chen.5i5j@gmail.com>

	gcc/
	PR target/71331
	* config/tilegx/tilegx.c (tilegx_function_profiler): Save r10
	to stack before call mcount.
	(tilegx_can_use_return_insn_p): Clean up code.
---
 gcc/config/tilegx/tilegx.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Hello Maintainers:

Is this patch OK? Please help check it when you have time (at least for
me, it passes all related test and comparation).

And I shall continue to find and fix another issues about tilegx.

Thanks.

On 6/4/16 21:25, chengang@emindsoft.com.cn wrote:
> From: Chen Gang <gang.chen.5i5j@gmail.com>
> 
> r10 may also be as parameter stack pointer for the nested function, so
> need save it before call mcount.
> 
> Also clean up code: use '!' instead of "== 0" for checking
> static_chain_decl and compute_total_frame_size.
> 
> 2016-06-04  Chen Gang  <gang.chen.5i5j@gmail.com>
> 
> 	gcc/
> 	PR target/71331
> 	* config/tilegx/tilegx.c (tilegx_function_profiler): Save r10
> 	to stack before call mcount.
> 	(tilegx_can_use_return_insn_p): Clean up code.
> ---
>  gcc/config/tilegx/tilegx.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/config/tilegx/tilegx.c b/gcc/config/tilegx/tilegx.c
> index 06c832c..55161bb 100644
> --- a/gcc/config/tilegx/tilegx.c
> +++ b/gcc/config/tilegx/tilegx.c
> @@ -3880,8 +3880,8 @@ bool
>  tilegx_can_use_return_insn_p (void)
>  {
>    return (reload_completed
> -	  && cfun->static_chain_decl == 0
> -	  && compute_total_frame_size () == 0
> +	  && !cfun->static_chain_decl
> +	  && !compute_total_frame_size ()
>  	  && tilegx_current_function_is_leaf ()
>  	  && !crtl->profile && !df_regs_ever_live_p (TILEGX_LINK_REGNUM));
>  }
> @@ -5507,6 +5507,15 @@ tilegx_function_profiler (FILE *file, int labelno ATTRIBUTE_UNUSED)
>        fprintf (file, "\t}\n");
>      }
>  
> +  if (cfun->static_chain_decl)
> +    {
> +      fprintf (file,
> +	       "\t{\n"
> +	       "\taddi\tsp, sp, -8\n"
> +	       "\tst\tsp, r10\n"
> +	       "\t}\n");
> +    }
> +
>    if (flag_pic)
>      {
>        fprintf (file,
> @@ -5524,6 +5533,13 @@ tilegx_function_profiler (FILE *file, int labelno ATTRIBUTE_UNUSED)
>  	       "\t}\n", MCOUNT_NAME);
>      }
>  
> +  if (cfun->static_chain_decl)
> +    {
> +      fprintf (file,
> +	       "\taddi\tsp, sp, 8\n"
> +	       "\tld\tr10, sp\n");
> +    }
> +
>    tilegx_in_bundle = false;
>  }
>  
>

On 10/06/2016 07:53 AM, Chen Gang wrote:
> Hello Maintainers:
>
> Is this patch OK? Please help check it when you have time (at least for
> me, it passes all related test and comparation).
>
> And I shall continue to find and fix another issues about tilegx.
>
> Thanks.
>
> On 6/4/16 21:25, chengang@emindsoft.com.cn wrote:
>> From: Chen Gang <gang.chen.5i5j@gmail.com>
>>
>> r10 may also be as parameter stack pointer for the nested function, so
>> need save it before call mcount.
>>
>> Also clean up code: use '!' instead of "== 0" for checking
>> static_chain_decl and compute_total_frame_size.
>>
>> 2016-06-04  Chen Gang  <gang.chen.5i5j@gmail.com>
>>
>> 	gcc/
>> 	PR target/71331
>> 	* config/tilegx/tilegx.c (tilegx_function_profiler): Save r10
>> 	to stack before call mcount.
>> 	(tilegx_can_use_return_insn_p): Clean up code.
So if I understand the tilegx architecture correctly, you're issuing the 
r10 save & sp adjustment as a bundle, and the restore & sp adjustment as 
a bundle.

The problem is the semantics of bunding on the tilegx effectively mean 
that all source operands are read in parallel, then all outputs occur in 
parallel.

So if we take the bundle

{addi sp,sp,-8 ; st sp, r10}

The address used for the st is the value of the stack pointer before the 
addi instruction.

Similarly for the restore r10 bundle.  The address used for the load is 
sp before adjustment.

Given my understanding of the tilegx bundling semantics, that seems wrong.

Jeff

On 10/20/16 06:42, Jeff Law wrote:
>> On 6/4/16 21:25, chengang@emindsoft.com.cn wrote:
>>> From: Chen Gang <gang.chen.5i5j@gmail.com>
>>>
>>> r10 may also be as parameter stack pointer for the nested function, so
>>> need save it before call mcount.
>>>
>>> Also clean up code: use '!' instead of "== 0" for checking
>>> static_chain_decl and compute_total_frame_size.
>>>
>>> 2016-06-04  Chen Gang  <gang.chen.5i5j@gmail.com>
>>>
>>>     gcc/
>>>     PR target/71331
>>>     * config/tilegx/tilegx.c (tilegx_function_profiler): Save r10
>>>     to stack before call mcount.
>>>     (tilegx_can_use_return_insn_p): Clean up code.
> So if I understand the tilegx architecture correctly, you're issuing the r10 save & sp adjustment as a bundle, and the restore & sp adjustment as a bundle.
> 
> The problem is the semantics of bunding on the tilegx effectively mean that all source operands are read in parallel, then all outputs occur in parallel.
> 
> So if we take the bundle
> 
> {addi sp,sp,-8 ; st sp, r10}
> 
> The address used for the st is the value of the stack pointer before the addi instruction.
> 
> Similarly for the restore r10 bundle.  The address used for the load is sp before adjustment.
> 
> Given my understanding of the tilegx bundling semantics, that seems wrong.
> 
> Jeff
>
 
The comments on 1st page of "TILE-Gx Instruction Set Architecture":

Individual instructions within a bundle must comply with certain register semantics. Read-after-write (RAW) dependencies are enforced between instruction bundles. There is no ordering within a bundle, and the numbering of pipelines or instruction slots within a bundle is only used for convenience and does not imply any ordering. Within an instruction bundle, it is valid to encode an output operand that is the same as an input operand. Because there is explicitly no implied dependency within a bundle, the semantics for this specify that the input operands for all instructions in a bundle are read before any of the output operands are written.

Write-after-write (WAW) semantics between two bundles are defined as: the latest write over-writes earlier writes.

Within a bundle, WAW dependencies are forbidden. If more than one instruction in a bundle writes to the same output operand register, unpredictable results for any destination operand within that bundle can occur. Also, implementations are free to signal this case as an illegal instruction. There is one exception to this rule—multiple instructions within a bundle may legally target the zero register. Lastly, some instructions, such as instructions that implicitly write the link register, implicitly write registers. If an instruction implicitly writes to a register that another instruction in the same bundle writes to, unpredictable results can occur for any output register used by that bundle and/or an illegal instruction interrupt can occur.

On Page 221, ld instruction is:

  ld Dest, Src

On Page 251, st instruction is:

  st SrcA, SrcB


So for me:

  Bundle {addi sp, sp, 8; ld r10, sp} is OK, it is RAW.

  Bundle {addi sp, sp, -8; st sp, r10} is OK, too, it is RAW (not WAW --
  both SrcA and SrcB are input operands).


Please help check, if need the related document, please let me know.

Thanks.

On 10/21/2016 6:24 PM, Chen Gang wrote:
> On 10/20/16 06:42, Jeff Law wrote:
>>> On 6/4/16 21:25, chengang@emindsoft.com.cn wrote:
>>>> From: Chen Gang <gang.chen.5i5j@gmail.com>
>>>>
>>>> r10 may also be as parameter stack pointer for the nested function, so
>>>> need save it before call mcount.
>>>>
>>>> Also clean up code: use '!' instead of "== 0" for checking
>>>> static_chain_decl and compute_total_frame_size.
>>>>
>>>> 2016-06-04  Chen Gang  <gang.chen.5i5j@gmail.com>
>>>>
>>>>      gcc/
>>>>      PR target/71331
>>>>      * config/tilegx/tilegx.c (tilegx_function_profiler): Save r10
>>>>      to stack before call mcount.
>>>>      (tilegx_can_use_return_insn_p): Clean up code.
>> So if I understand the tilegx architecture correctly, you're issuing the r10 save & sp adjustment as a bundle, and the restore & sp adjustment as a bundle.
>>
>> The problem is the semantics of bunding on the tilegx effectively mean that all source operands are read in parallel, then all outputs occur in parallel.
>>
>> So if we take the bundle
>>
>> {addi sp,sp,-8 ; st sp, r10}
>>
>> The address used for the st is the value of the stack pointer before the addi instruction.
>>
>> Similarly for the restore r10 bundle.  The address used for the load is sp before adjustment.
>>
>> Given my understanding of the tilegx bundling semantics, that seems wrong.
>>
>> Jeff
>>
>   
> The comments on 1st page of "TILE-Gx Instruction Set Architecture":
>
> Individual instructions within a bundle must comply with certain register semantics. Read-after-write (RAW) dependencies are enforced between instruction bundles. There is no ordering within a bundle, and the numbering of pipelines or instruction slots within a bundle is only used for convenience and does not imply any ordering. Within an instruction bundle, it is valid to encode an output operand that is the same as an input operand. Because there is explicitly no implied dependency within a bundle, the semantics for this specify that the input operands for all instructions in a bundle are read before any of the output operands are written.
>
> Write-after-write (WAW) semantics between two bundles are defined as: the latest write over-writes earlier writes.
>
> Within a bundle, WAW dependencies are forbidden. If more than one instruction in a bundle writes to the same output operand register, unpredictable results for any destination operand within that bundle can occur. Also, implementations are free to signal this case as an illegal instruction. There is one exception to this rule—multiple instructions within a bundle may legally target the zero register. Lastly, some instructions, such as instructions that implicitly write the link register, implicitly write registers. If an instruction implicitly writes to a register that another instruction in the same bundle writes to, unpredictable results can occur for any output register used by that bundle and/or an illegal instruction interrupt can occur.
>
> On Page 221, ld instruction is:
>
>    ld Dest, Src
>
> On Page 251, st instruction is:
>
>    st SrcA, SrcB
>
>
> So for me:
>
>    Bundle {addi sp, sp, 8; ld r10, sp} is OK, it is RAW.
>
>    Bundle {addi sp, sp, -8; st sp, r10} is OK, too, it is RAW (not WAW --
>    both SrcA and SrcB are input operands).
>
>
> Please help check, if need the related document, please let me know.

As you wrote, RAW applies "between instruction bundles".  In this case you are looking at register usage within a single bundle, and as you wrote, "the input operands for all instructions in a bundle are read before any of the output operands are written."  So for your two bundles quoted above, the "sp" input operand for both instructions will have the same value, i.e. the load/store will have the pre-adjusted "sp" value.