[Patch 1/8, Arm, AArch64, GCC] Refactor mbranch-protection option parsing and make it common to AArch32 and AArch64 backends. [Was RE: [Patch 2/7, Arm, GCC] Add option -mbranch-protection.]

Andrea Corallo andrea.corallo@arm.com
Wed Nov 10 13:55:02 GMT 2021


Tejas Belagod via Gcc-patches <gcc-patches@gcc.gnu.org> writes:

[...]

> This change refactors all the mbranch-protection option parsing code and types
> to make it common to both AArch32 and AArch64 backends.  This change also pulls
> in some supporting types from AArch64 to make it common
> (aarch_parse_opt_result).  The significant changes in this patch are the
> movement of all branch protection parsing routines from aarch64.c to
> aarch-common.c and supporting data types and static data structures.  This
> patch also pre-declares variables and types required in the aarch32 back for
> moved variables for function sign scope and key to prepare for the impending
> series of patches that support parsing the feature mbranch-protection in the
> aarch32 back end.
>
> 2021-10-25  Tejas Belagod  <tbelagod@arm.com>
>
> gcc/ChangeLog:
>
> 	* common/config/aarch64/aarch64-common.c: Include aarch-common.h.
> 	(all_architectures): Fix comment.
> 	(aarch64_parse_extension): Rename return type, enum value names.
> 	* config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): Rename
> 	factored out aarch_ra_sign_scope and aarch_ra_sign_key variables.
> 	Also rename corresponding enum values.
> 	* config/aarch64/aarch64-opts.h (aarch64_function_type): Factor out
> 	aarch64_function_type and move it to common code as aarch_function_type
> 	in aarch-common.h.
> 	* config/aarch64/aarch64-protos.h: Include common types header, move out
> 	types aarch64_parse_opt_result and aarch64_key_type to aarch-common.h
> 	* config/aarch64/aarch64.c: Move mbranch-protection parsing types and
> 	functions out into aarch-common.h and aarch-common.c.  Fix up all the name
> 	changes resulting from the move.
> 	* config/aarch64/aarch64.md: Fix up aarch64_ra_sign_key type name change
> 	and enum value.
> 	* config/aarch64/aarch64.opt: Include aarch-common.h to import type move.
> 	Fix up name changes from factoring out common code and data.
> 	* config/arm/aarch-common-protos.h: Export factored out routines to both
> 	backends.
> 	* config/arm/aarch-common.c: Include newly factored out types.  Move all
> 	mbranch-protection code and data structures from aarch64.c.
> 	* config/arm/aarch-common.h: New header that declares types shared between
> 	aarch32 and aarch64 backends.
> 	* config/arm/arm-protos.h: Declare types and variables that are made common
> 	to aarch64 and aarch32 backends - aarch_ra_sign_key, aarch_ra_sign_scope and
> 	aarch_enable_bti.
>
>
> Tested the following configurations. OK for trunk?
>
> -mthumb/-march=armv8.1-m.main+pacbti/-mfloat-abi=soft
> -marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp
> mcmodel=small and tiny
> aarch64-none-linux-gnu native test and bootstrap
>
> Thanks,
> Tejas.

Hi Tejas,

going through the code I've spotted a couple of indentation nits that I
guess are coming from the original source that was moved.

> diff --git a/gcc/config/arm/aarch-common.c b/gcc/config/arm/aarch-common.c

[...]

> +  /* Copy the last processed token into the argument to pass it back.
> +    Used by option and attribute validation to print the offending token.  */
> +  if (last_str)
> +    {
> +      if (str) strcpy (*last_str, str);
> +      else *last_str = NULL;

I think we should have new lines after both if and else here.

> +    }
> +  if (res == AARCH_PARSE_OK)
> +    {
> +      /* If needed, alloc the accepted string then copy in const_str.
> +	Used by override_option_after_change_1.  */
> +      if (!accepted_branch_protection_string)
> +	accepted_branch_protection_string = (char *) xmalloc (
> +						      BRANCH_PROTECT_STR_MAX
> +							+ 1);
						        ^^
                                                        Indentation


> +      strncpy (accepted_branch_protection_string, const_str,
> +		BRANCH_PROTECT_STR_MAX + 1);
		^^
                Same
> +      /* Forcibly null-terminate.  */
> +      accepted_branch_protection_string[BRANCH_PROTECT_STR_MAX] = '\0';
> +    }
> +  return res;
> +}

Thanks

  Andrea


More information about the Gcc-patches mailing list