[PATCH, gas/ARM] Clean up selection of feature bits

[Thomas Preudhomme]

Hi Nick,

Thank you for the approval and the quick review. I've noticed one more issue in 
mark_feature_used which was referring to selected architecture but testing 
cpu_variant (which only represents what is allowed). I've considered this to be 
an obvious fix so have committed the patch with that comment fix.

You'll find attached the patch I've committed. As I said, the only difference is 
in mark_feature_used so just search for that function if you want to double check.

Updated ChangeLog entry is as follows:

*** gas/ChangeLog ***

2018-03-01  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	* config/tc-arm.c (cpu_variant, arm_arch_used, thumb_arch_used,
	legacy_cpu, legacy_fpu, mcpu_cpu_opt, dyn_mcpu_ext_opt,
	mcpu_fpu_opt, march_cpu_opt, dyn_march_ext_opt, march_fpu_opt,
	mfpu_opt, object_arch, selected_cpu): Comment meaning of variables.
	(dyn_mcpu_ext_opt): Also rename into ...
	(mcpu_ext_opt): This.
	(dyn_march_ext_opt): Also rename into ...
	(march_ext_opt): This.
	(object_arch): Also rename into ...
	(selected_object_arch): This and make it a plain arm_feature_set
	structure.
	(selected_arch, selected_ext, selected_fpu): New static variables.
	(mark_feature_used): Fix comments, feature is marked as used iff
	it is currently allowed.
	(do_bx): Adapt to change in name and type of object_arch.
	(md_begin): Set selected_arch rather than mcpu_cpu_opt, selected_ext
	rather than dyn_mcpu_ext_opt and selected_fpu rather than mfpu_opt.
	Remove dead code to set default FPU if architecture version is greater
	than 5.  Set all CPU bits of cpu_variant directly in autodection
	leaving mcpu_cpu_opt, selected_arch and selected_cpu unset.
	(arm_parse_extension): Take extension feature set pointer
	parameter by value rather than by pointer.  Remove allocation
	code.  Adapt code accordingly.
	(arm_parse_cpu): Adapt to variable renaming and changes in
	arm_parse_extension () signature.
	(arm_parse_arch): Likewise.
	(aeabi_set_public_attributes): Also set selected_arch and selected_ext
	in addition to selected_cpu.  Set flags_arch and flags_ext from them
	instead of selected_cpu.  Adapt to variables renaming and type change.
	(arm_md_post_relax): Adapt to variable renaming.
	(s_arm_cpu): Set selcted_cpu_cpu and selected_ext instead of
	mcpu_cpu_opt and dyn_mcpu_ext_opt.  Set selected_cpu from them
	and cpu_variant from selected_cpu and selected_fpu.
	(s_arm_arch): Likewise.
	(s_arm_object_arch): Adapt to variable renaming.
	(s_arm_arch_extension): Use ARM_CPU_IS_ANY instead of checking
	feature set against arm_any.  Check selected_arch rather than
	*mcpu_cpu_opt.  Set selected_ext rather than *dyn_mcpu_ext_opt and
	remove allocation code.
	(s_arm_fpu): Set selected_fpu instead of mfpu_opt.  Set all CPU
	feature bits if in autodetection mode.

Best regards,

Thomas

On 01/03/18 10:58, Nick Clifton wrote:
> Hi Thomas,
> 
>> I've always found the code in ARM backend of gas to control what
>> CPU/architecture and FPU are selected by the user and to support
>> autodetection of features complex and confusing. Chief among the
>> issues I have with that code is the lack of comments to explain
>> the meaning of the various variables. This patch addresses that
>> and much more:
> 
> Wow - thank you for doing this.
> 
> My first thought actually was whether this improvement ought to be generalized
> and used for other targets, and not just ARM.  I think however that there is
> probably too much target specific knowledge to be handled, but it might be worth
> thinking about for the future.
> 
>> *** gas/ChangeLog ***
>> 2018-02-21  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>  Â Â Â Â * config/tc-arm.c (cpu_variant, arm_arch_used, thumb_arch_used,
>>  Â Â Â Â legacy_cpu, legacy_fpu, mcpu_cpu_opt, dyn_mcpu_ext_opt,
>>  Â Â Â Â mcpu_fpu_opt, march_cpu_opt, dyn_march_ext_opt, march_fpu_opt,
>>  Â Â Â Â mfpu_opt, object_arch, selected_cpu): Comment meaning of variables.
>>  Â Â Â Â (dyn_mcpu_ext_opt): Also rename into ...
>>  Â Â Â Â (mcpu_ext_opt): This.
>>  Â Â Â Â (dyn_march_ext_opt): Also rename into ...
>>  Â Â Â Â (march_ext_opt): This.
>>  Â Â Â Â (object_arch): Also rename into ...
>>  Â Â Â Â (selected_object_arch): This and make it a plain arm_feature_set
>>  Â Â Â Â structure.
>>  Â Â Â Â (selected_arch, selected_ext, selected_fpu): New static variables.
>>  Â Â Â Â (do_bx): Adapt to change in name and type of object_arch.
>>  Â Â Â Â (md_begin): Set selected_arch rather than mcpu_cpu_opt, selected_ext
>>  Â Â Â Â rather than dyn_mcpu_ext_opt and selected_fpu rather than mfpu_opt.
>>  Â Â Â Â Remove dead code to set default FPU if architecture version is greater
>>      than 5.  Set all CPU bits of cpu_variant directly in autodection
>>  Â Â Â Â leaving mcpu_cpu_opt, selected_arch and selected_cpu unset.
>>  Â Â Â Â (arm_parse_extension): Take extension feature set pointer
>>      parameter by value rather than by pointer.  Remove allocation
>>      code.  Adapt code accordingly.
>>  Â Â Â Â (arm_parse_cpu): Adapt to variable renaming and changes in
>>  Â Â Â Â arm_parse_extension () signature.
>>  Â Â Â Â (arm_parse_arch): Likewise.
>>  Â Â Â Â (aeabi_set_public_attributes): Also set selected_arch and selected_ext
>>      in addition to selected_cpu.  Set flags_arch and flags_ext from them
>>      instead of selected_cpu.  Adapt to variables renaming and type change.
>>  Â Â Â Â (arm_md_post_relax): Adapt to variable renaming.
>>  Â Â Â Â (s_arm_cpu): Set selcted_cpu_cpu and selected_ext instead of
>>      mcpu_cpu_opt and dyn_mcpu_ext_opt.  Set selected_cpu from them
>>  Â Â Â Â and cpu_variant from selected_cpu and selected_fpu.
>>  Â Â Â Â (s_arm_arch): Likewise.
>>  Â Â Â Â (s_arm_object_arch): Adapt to variable renaming.
>>  Â Â Â Â (s_arm_arch_extension): Use ARM_CPU_IS_ANY instead of checking
>>      feature set against arm_any.  Check selected_arch rather than
>>      *mcpu_cpu_opt.  Set selected_ext rather than *dyn_mcpu_ext_opt and
>>  Â Â Â Â remove allocation code.
>>      (s_arm_fpu): Set selected_fpu instead of mfpu_opt.  Set all CPU
>>  Â Â Â Â feature bits if in autodetection mode.
> 
> Approved - please apply.
> 
> Cheers
>    Nick
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cleanup_selection_feature_bits.patch
Type: text/x-patch
Size: 17275 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20180301/687af945/attachment.bin>