[Patch]Extend GAS arm_feature_set struct to provide more available bits

Nicholas Clifton nickc@redhat.com
Tue Mar 10 11:04:00 GMT 2015


Hi Terry,

> Currently all 32 bits in core filed of struct arm_feature_set are occupied
> to represent various features.

wouldn't it have been easier to just switch to a 64-bit data type ? 
Just asking...


> include/opcode/ChangeLog:
> 2015-03-05  Terry Guo  <terry.guo@arm.com>
>
>      * arm.h (arm_feature_set): Redefined to provide more available bits.
>      (ARM_ANY): Updated to follow above new definition.
>      (ARM_CPU_HAS_FEATURE): Likewise.
>      (ARM_CPU_IS_ANY): Likewise.
>      (ARM_MERGE_FEATURE_SETS): Likewise.
>      (ARM_CLEAR_FEATURE): Likewise.
>      (ARM_FEATURE): Likewise.
>      (ARM_FEATURE_COPY): New macro.
>      (ARM_FEATURE_EQUAL): Likewise.
>      (ARM_FEATURE_ZERO): Likewise.
>      (ARM_FEATURE_CORE_EQUAL): Likewise.
>      (ARM_FEATURE_LOW): Likewise.
>
> gas/ChangeLog:
> 2015-03-05  Terry Guo  <terry.guo@arm.com>
>
>      * config/tc-arm.c (no_cpu_selected): Use new macro to compare features.
>      (parse_psr): Likewise.
>      (do_t_mrs): Likewise.
>      (do_t_msr): Likewise.
>
> opcodes/ChangeLog:
> 2015-03-05  Terry Guo  <terry.guo@arm.com>
>
>      * arm-dis.c (opcode32): Updated to use new arm feature struct.
>      (opcode16): Likewise.
>      (coprocessor_opcodes): Replace bit with feature struct.
>      (neon_opcodes): Likewise.
>      (arm_opcodes): Likewise.
>      (thumb_opcodes): Likewise.
>      (thumb32_opcodes): Likewise.
>      (print_insn_coprocessor): Likewise.
>      (print_insn_arm): Likewise.
>      (select_arm_features): Follow new feature struct.

Approved - please apply - but...

The current macro method seems a bit messy, especially now that there 
are so many features to encode and test.  I would have preferred to see 
a rewrite to a more extensible mechanism that could cope with an 
unlimited(*) number of features without the need for future updates.

That said, if you stick with your current patch, I noticed that there a 
lots of places where ARM_FEATURE_LOW is called with a 0 as its first or 
second arguments.  Since you are already renaming the macro why not 
create a couple of new ones that eliminate the need for a redundant 
argument ?  Eg:

   #define ARM_FEATURE_CORE_LOW(core)   {{(core), 0}, 0}
   #define ARM_FEATURE_COPROC(coproc)   {{0, 0}, (coproc)}

It just makes the code look that little bit cleaner.

Cheers
   Nick

(*) For some reasonably small value of unlimited, eg 1^32.



More information about the Binutils mailing list