This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Allow MIPS16 symbols to have nondefault visiblity
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Nick Clifton <nickc at redhat dot com>
- Cc: binutils at sourceware dot org
- Date: Mon, 07 Jul 2008 20:15:50 +0100
- Subject: Re: [PATCH] Allow MIPS16 symbols to have nondefault visiblity
- References: <87zlp51qo5.fsf@firetop.home> <487240E7.3030503@redhat.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