Re: [PATCH] Allow MIPS16 symbols to have nondefault visiblity

[Richard Sandiford <rdsandiford at googlemail dot com>]

Nick Clifton <nickc@redhat.com> writes:
> Hi Richard,
>> include/elf/
>> 	* mips.h (ELF_ST_IS_MIPS16, ELF_ST_SET_MIPS16): New macros.
>> 
>> bfd/
>> 	* elfxx-mips.c (mips_elf_check_mips16_stubs): Use ELF_ST_IS_MIPS16.
>> 	(mips_elf_calculate_relocation): Likewise.
>> 	(_bfd_mips_elf_add_symbol_hook): Likewise.
>> 	(_bfd_mips_elf_finish_dynamic_symbol): Likewise.
>> 	(_bfd_mips_vxworks_finish_dynamic_symbol): Likewise.
>> 
>> opcodes/
>> 	* mips-dis.c (_print_insn_mips): Use ELF_ST_IS_MIPS16.
>> 
>> gas/
>> 	* config/tc-mips.c (mips16_mark_labels): Use ELF_ST_SET_MIPS16.
>> 	(mips_fix_adjustable): Likewise.
>> 	(mips_frob_file_after_relocs): Likewise.
>> 
>> gas/testsuite/
>> 	* gas/mips/mips16-vis-1.d, gas/mips/mips16-vis-1.s: New tests.
>> 	* gas/mips/mips.exp: Run them.
>
> Approved - please apply - but ...
>
>>  /* This value is used for a mips16 .text symbol.  */
>>  #define STO_MIPS16		0xf0
>> +#define ELF_ST_IS_MIPS16(OTHER) (((OTHER) & 0xf0) == STO_MIPS16)
>> +#define ELF_ST_SET_MIPS16(OTHER) (((OTHER) & ~0xf0) | STO_MIPS16)
>
> Wouldn't it make more sense to use STO_MIPS16 instead of 0xf0 in these 
> new macros ?  ie:
>
>    #define ELF_ST_IS_MIPS16(OTHER) (((OTHER) & STO_MIPS16) == STO_MIPS16)

Ah, sorry, I should have mentioned that.  This ties in with later patches,
which treated the top four bits of st_other as an enum.  It looks like
we might still be doing that for any future STO_MIPS_PIC flag.

Stepping back, the problem is that STO_MIPS16 rather rudely takes up
half of st_other, and we currently need one or two more STO_* values.
It would be good to put mutually-exclusive STO_* values in the same bits.

I couldn't think of a good name for the enum, so I didn't introduce
a mnemonic for it...

Richard