[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