[PATCH, ARM] Support for value 3 of Tag_ABI_VFP_args ARM attribute

[PATCH, ARM] Support for value 3 of Tag_ABI_VFP_args ARM attribute

[Thomas Preud'homme]

Hi there,

By mistake a small gdb patch was commited while commiting the binutils patch to support value 3 of Tag_ABI_VFP_args ARM attribute because I forgot to split the patch into two parts. My sincere apologize, this is my first binutils-gdb patch and the combined repository but double submission confused me. The patch is very straightforward so I hope it's not a problem but if it is I'll promptly revert it of course. The following gdb change was commited:

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 43520cc..a917862 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -10007,27 +10007,34 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 							OBJ_ATTR_PROC,
 							Tag_ABI_VFP_args))
 			{
-			case 0:
+			case (int) AEABI_VFP_args_base:
 			  /* "The user intended FP parameter/result
 			     passing to conform to AAPCS, base
 			     variant".  */
 			  fp_model = ARM_FLOAT_SOFT_VFP;
 			  break;
-			case 1:
+			case (int) AEABI_VFP_args_vfp:
 			  /* "The user intended FP parameter/result
 			     passing to conform to AAPCS, VFP
 			     variant".  */
 			  fp_model = ARM_FLOAT_VFP;
 			  break;
-			case 2:
+			case (int) AEABI_VFP_args_toolchain:
 			  /* "The user intended FP parameter/result
 			     passing to conform to tool chain-specific
 			     conventions" - we don't know any such
 			     conventions, so leave it as "auto".  */
 			  break;
+			case (int) AEABI_VFP_args_compatible:
+			  /* "Code is compatible with both the base
+			     and VFP variants; the user did not permit
+			     non-variadic functions to pass FP
+			     parameters/results" - leave it as
+			     "auto".  */
+			  break;
 			default:
 			  /* Attribute value not mentioned in the
-			     October 2008 ABI, so leave it as
+			     November 2012 ABI, so leave it as
 			     "auto".  */
 			  break;
 			}

The definition was added in include/elf/arm.h in the same commit as follows:

diff --git a/include/elf/arm.h b/include/elf/arm.h
index 34afdfd..e85536b 100644
--- a/include/elf/arm.h
+++ b/include/elf/arm.h
@@ -319,6 +319,23 @@ enum
   Tag_VFP_HP_extension = Tag_FP_HP_extension
 };
 
+/* Values for Tag_ABI_FP_number_model.  */
+enum
+{
+  AEABI_FP_number_model_none = 0,
+  AEABI_FP_number_model_ieee754_number = 1,
+  AEABI_FP_number_model_rtabi = 2,
+  AEABI_FP_number_model_ieee754_all = 3
+};
+
+/* Values for Tag_ABI_VFP_args.  */
+enum
+{
+  AEABI_VFP_args_base = 0,
+  AEABI_VFP_args_vfp = 1,
+  AEABI_VFP_args_toolchain = 2,
+  AEABI_VFP_args_compatible = 3
+};
 #endif
 
 /* The name of the note section used to identify arm variants.  */

Please let me know if I should revert the patch and sorry again for the trouble.

Best regards,

Thomas

Re: [PATCH, ARM] Support for value 3 of Tag_ABI_VFP_args ARM attribute

[Joel Brobecker]

> By mistake a small gdb patch was commited while commiting the binutils
> patch to support value 3 of Tag_ABI_VFP_args ARM attribute because I
> forgot to split the patch into two parts. My sincere apologize, this
> is my first binutils-gdb patch and the combined repository but double
> submission confused me. The patch is very straightforward so I hope
> it's not a problem but if it is I'll promptly revert it of course. The
> following gdb change was commited:

No problem.

You forgot to provide the ChangeLog entry.  FTR, here it is:

2014-12-25  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * arm-tdep.c (arm_gdbarch_init): Explicitly handle value 3 of
        Tag_ABI_VFP_args. Also replace hardcoded values by enum values in the
        switch handling the different values of Tag_ABI_VFP_args.

Also, I don't think the cast to (int) are necessary, are they?

> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 43520cc..a917862 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -10007,27 +10007,34 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>  							OBJ_ATTR_PROC,
>  							Tag_ABI_VFP_args))
>  			{
> -			case 0:
> +			case (int) AEABI_VFP_args_base:
>  			  /* "The user intended FP parameter/result
>  			     passing to conform to AAPCS, base
>  			     variant".  */
>  			  fp_model = ARM_FLOAT_SOFT_VFP;
>  			  break;
> -			case 1:
> +			case (int) AEABI_VFP_args_vfp:
>  			  /* "The user intended FP parameter/result
>  			     passing to conform to AAPCS, VFP
>  			     variant".  */
>  			  fp_model = ARM_FLOAT_VFP;
>  			  break;
> -			case 2:
> +			case (int) AEABI_VFP_args_toolchain:
>  			  /* "The user intended FP parameter/result
>  			     passing to conform to tool chain-specific
>  			     conventions" - we don't know any such
>  			     conventions, so leave it as "auto".  */
>  			  break;
> +			case (int) AEABI_VFP_args_compatible:
> +			  /* "Code is compatible with both the base
> +			     and VFP variants; the user did not permit
> +			     non-variadic functions to pass FP
> +			     parameters/results" - leave it as
> +			     "auto".  */
> +			  break;
>  			default:
>  			  /* Attribute value not mentioned in the
> -			     October 2008 ABI, so leave it as
> +			     November 2012 ABI, so leave it as
>  			     "auto".  */
>  			  break;
>  			}
> 
> The definition was added in include/elf/arm.h in the same commit as follows:
> 
> diff --git a/include/elf/arm.h b/include/elf/arm.h
> index 34afdfd..e85536b 100644
> --- a/include/elf/arm.h
> +++ b/include/elf/arm.h
> @@ -319,6 +319,23 @@ enum
>    Tag_VFP_HP_extension = Tag_FP_HP_extension
>  };
>  
> +/* Values for Tag_ABI_FP_number_model.  */
> +enum
> +{
> +  AEABI_FP_number_model_none = 0,
> +  AEABI_FP_number_model_ieee754_number = 1,
> +  AEABI_FP_number_model_rtabi = 2,
> +  AEABI_FP_number_model_ieee754_all = 3
> +};
> +
> +/* Values for Tag_ABI_VFP_args.  */
> +enum
> +{
> +  AEABI_VFP_args_base = 0,
> +  AEABI_VFP_args_vfp = 1,
> +  AEABI_VFP_args_toolchain = 2,
> +  AEABI_VFP_args_compatible = 3
> +};
>  #endif
>  
>  /* The name of the note section used to identify arm variants.  */
> 
> Please let me know if I should revert the patch and sorry again for the trouble.
> 
> Best regards,
> 
> Thomas
> 
> 

-- 
Joel

RE: [PATCH, ARM] Support for value 3 of Tag_ABI_VFP_args ARM attribute

[Thomas Preud'homme]

> From: Joel Brobecker [mailto:brobecker@adacore.com]
> Sent: Sunday, December 28, 2014 3:47 AM
> 
> No problem.
> 
> You forgot to provide the ChangeLog entry.  FTR, here it is:
> 
> 2014-12-25  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * arm-tdep.c (arm_gdbarch_init): Explicitly handle value 3 of
>         Tag_ABI_VFP_args. Also replace hardcoded values by enum values in
> the
>         switch handling the different values of Tag_ABI_VFP_args.

Thanks.

> 
> Also, I don't think the cast to (int) are necessary, are they?

Sigh. Indeed, they aren't. My apologize. I remember adding them after seeing an error but I cannot reproduce it. So either I compiled with g++ (why would I?) or the error came from something else in the patch which I changed as well. Should I post a patch to fix this or can I commit it as obvious?

Best regards,

Thomas

Re: [PATCH, ARM] Support for value 3 of Tag_ABI_VFP_args ARM attribute

[Joel Brobecker]

> > Also, I don't think the cast to (int) are necessary, are they?
> 
> Sigh. Indeed, they aren't. My apologize. I remember adding them after
> seeing an error but I cannot reproduce it. So either I compiled with
> g++ (why would I?) or the error came from something else in the patch
> which I changed as well. Should I post a patch to fix this or can I
> commit it as obvious?

No worries at all - if you had seen the number of brainless changes
I've pushed (and I am not calling your change brainless ;-)), you
would feel sorry for me.

You can commit that change as obvious, no problem. But, in case we
haven't said it before, even obvious changes need to be posted here,
with ChangeLog and all.

Thank you,
-- 
Joel

RE: [PATCH, ARM] Support for value 3 of Tag_ABI_VFP_args ARM attribute

[Thomas Preud'homme]

> From: Joel Brobecker [mailto:brobecker@adacore.com]
> Sent: Monday, December 29, 2014 12:09 PM
> 

> You can commit that change as obvious, no problem. But, in case we
> haven't said it before, even obvious changes need to be posted here,
> with ChangeLog and all.
> 

Commited by courtesy of my colleague Jiong Wang.

ChangeLog entry is as follows:

*** gdb/ChangeLog ***

2014-12-29 Thomas Preud'homme thomas.preudhomme@arm.com

    * arm-tdep.c (arm_gdbarch_init): Remove casts in Tag_ABI_VFP_args
    switch case statements.


diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index e12ae6e..dfec923 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -9968,25 +9968,25 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 							OBJ_ATTR_PROC,
 							Tag_ABI_VFP_args))
 			{
-			case (int) AEABI_VFP_args_base:
+			case AEABI_VFP_args_base:
 			  /* "The user intended FP parameter/result
 			     passing to conform to AAPCS, base
 			     variant".  */
 			  fp_model = ARM_FLOAT_SOFT_VFP;
 			  break;
-			case (int) AEABI_VFP_args_vfp:
+			case AEABI_VFP_args_vfp:
 			  /* "The user intended FP parameter/result
 			     passing to conform to AAPCS, VFP
 			     variant".  */
 			  fp_model = ARM_FLOAT_VFP;
 			  break;
-			case (int) AEABI_VFP_args_toolchain:
+			case AEABI_VFP_args_toolchain:
 			  /* "The user intended FP parameter/result
 			     passing to conform to tool chain-specific
 			     conventions" - we don't know any such
 			     conventions, so leave it as "auto".  */
 			  break;
-			case (int) AEABI_VFP_args_compatible:
+			case AEABI_VFP_args_compatible:
 			  /* "Code is compatible with both the base
 			     and VFP variants; the user did not permit
 			     non-variadic functions to pass FP

Best regards,

Thomas