Re: Incorrect code due to indirect tail call of varargs function with hard float ABI

[Ramana Radhakrishnan <ramana.radhakrishnan@foss.arm.com>]

On 18/11/15 00:32, Kugan wrote:
>> > Hi Ramana,
>> > 
>> > Thanks for the review. I have opened a gcc bug-report for this. I tested
>> > the attached patch for  arm-none-linux-gnueabihf and
>> > arm-none-linux-gnueabi with no new regressions. Is this OK?
>> > 
>> > 
>> > Thanks,
>> > Kugan
>> > 
>> > gcc/ChangeLog:
>> > 
>> > 2015-11-18  Kugan Vivekanandarajah  <kuganv@linaro.org>
>> > 
>> > 	PR target/68390
>> > 	* config/arm/arm.c (arm_function_ok_for_sibcall): Get function type
>> > 	for indirect function call.
>> > 
>> > gcc/testsuite/ChangeLog:
>> > 
>> > 2015-11-18  Kugan Vivekanandarajah  <kuganv@linaro.org>
>> > 
>> > 	PR target/68390
>> > 	* gcc.target/arm/PR68390.c: New test.
>> > 
>> > 
> Hi Ramana,
> 
> With further testing on bare-metal, I found that for the following decl
> has to be null for indirect functions.
> 
>   if (TARGET_AAPCS_BASED
>       && arm_abi == ARM_ABI_AAPCS
>       && decl
>       && DECL_WEAK (decl))
>     return false;

Ok .. yes that's right.

> 
> Here is the updated patch and ChangeLog. Sorry for the noise.
> 
> Thanks,
> Kugan
> 
> 
> gcc/ChangeLog:
> 
> 2015-11-18  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	PR target/68390
> 	* config/arm/arm.c (arm_function_ok_for_sibcall): Get function type
> 	for indirect function call.
> 
> gcc/testsuite/ChangeLog:
> 
> 2015-11-18  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	PR target/68390
> 	* gcc.target/arm/PR68390.c: New test.
> 

s/PR/pr in the test name and put this in gcc.c-torture/execute instead - there is nothing ARM specific about the test. Tests in gcc.target/arm should really only be architecture specific. This isn't.

> 
> 
> 
> p.txt
> 
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index a379121..0dae7da 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -6680,8 +6680,13 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
>  	 a VFP register but then need to transfer it to a core
>  	 register.  */
>        rtx a, b;
> +      tree fn_decl = decl;

Call it decl_or_type instead - it's really that ... 

>  
> -      a = arm_function_value (TREE_TYPE (exp), decl, false);
> +      /* If it is an indirect function pointer, get the function type.  */
> +      if (!decl)
> +	fn_decl = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp)));
> +

This is probably just my mail client - but please watch out for indentation.

> +      a = arm_function_value (TREE_TYPE (exp), fn_decl, false);
>        b = arm_function_value (TREE_TYPE (DECL_RESULT (cfun->decl)),
>  			      cfun->decl, false);
>        if (!rtx_equal_p (a, b))


OK with those changes.

Ramana