This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH 1/2] MIPS: Compressed PLT/stubs support
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: "Maciej W. Rozycki" <macro at codesourcery dot com>
- Cc: Catherine Moore <clm at codesourcery dot com>, <binutils at sourceware dot org>
- Date: Thu, 06 Jun 2013 21:25:03 +0100
- Subject: Re: [PATCH 1/2] MIPS: Compressed PLT/stubs support
- References: <alpine dot DEB dot 1 dot 10 dot 1302072108300 dot 6762 at tp dot orcam dot me dot uk> <87621mwt3l dot fsf at talisman dot default> <alpine dot DEB dot 1 dot 10 dot 1303081151260 dot 21335 at tp dot orcam dot me dot uk> <87ppz9yq4z dot fsf at talisman dot default> <alpine dot DEB dot 1 dot 10 dot 1306062012320 dot 16287 at tp dot orcam dot me dot uk>
"Maciej W. Rozycki" <macro@codesourcery.com> writes:
>> The same 0xc0 bug exists here; the pre-microMIPS version was:
>>
>> #define ELF_ST_SET_MIPS_PIC(OTHER) \
>> (STO_MIPS_PIC | ELF_ST_VISIBILITY (OTHER))
>>
>> which I think was correct at the time. I'd suggest:
>>
>> #define ELF_ST_SET_MIPS_PIC(OTHER) \
>> ((ELF_ST_IS_MICROMIPS (OTHER) ? STO_MICROMIPS : 0) \
>> | ELF_ST_VISIBILITY (OTHER) \
>> | STO_MIPS_PIC)
>>
>> which is OK if you agree. The change to ELF_ST_IS_MIPS_PIC looks
>> superfluous though.
>
> I agree, however I'd prefer STO_MIPS16 to be referred explicitly here.
> Using STO_MICROMIPS as a shortcut where what we really mean is STO_MIPS16
> is only going to confuse people I believe.
Well, I disagree that's what the condition is doing. It's making a
positive statement that microMIPSness carries over, even though the
other currently-defined MIPS information doesn't.
> Also the macro would have to get tweaked further if 0x40 was defined
> as a valid STO_MIPS_ISA encoding in the future even if the meaning of
> STO_MIPS_PIC remained the same for that ISA.
True, but conversely, it would need to be tweaked the other way if
the new ISA encoding shouldn't be preserved. We just can't say now
which might be right.
But those arguments (of mine) are for one version not being inherently
better than the other. Since you've been flexible, I should be too...
> 2013-06-06 Maciej W. Rozycki <macro@codesourcery.com>
>
> include/elf/
> * mips.h (ELF_ST_SET_MIPS_PIC): Clear any STO_MIPS16 setting.
OK, thanks.
Richard