This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] MIPS/GAS: Complete constant JMP relocs straight away
On Sun, 23 Sep 2012, Richard Sandiford wrote:
> > That could be done. An artificial absolute symbol would have to be
> > created and assigned the constant referred. The relocation would then
> > refer to that symbol and remain for the linker to resolve and check the
> > range while doing that. But is it worth the effort? If so, then please
> > go ahead, sure!
>
> Probably stating the obvious, but that would only be needed for REL targets.
> RELA could just emit a relocation against symbol zero without losing bits.
Zero or not you need to emit that extra symbol and for the sake of
consistency I'd lean towards doing that the same way regardless of the
reloc style. It's not like we're going to have a scaling problem here.
> I agree that we should be consistent though, and either find a way of
> making it work for REL targets or not do it at all.
>
> I think the same argument applies to:
>
> case BFD_RELOC_16_PCREL_S2:
> {
> int shift;
>
> shift = mips_opts.micromips ? 1 : 2;
> if ((address_expr->X_add_number & ((1 << shift) - 1)) != 0)
> as_bad (_("branch to misaligned address (0x%lx)"),
> (unsigned long) address_expr->X_add_number);
> if (!mips_relax_branch)
> {
> if ((address_expr->X_add_number + (1 << (shift + 15)))
> & ~((1 << (shift + 16)) - 1))
> as_bad (_("branch address range overflow (0x%lx)"),
> (unsigned long) address_expr->X_add_number);
> ip->insn_opcode |= ((address_expr->X_add_number >> shift)
> & 0xffff);
> }
> ip->complete_p = 0;
> }
>
> In principle, we should either resolve the operand now and set
> complete_p to true, or not install any operand and create a fixup.
There's some non-trivial post-processing of this reloc in md_apply_fix,
although interestingly enough for the common case it only repeats what's
already done here -- the use of OR luckily guarantees the same result when
the same value is applied again.
I agree this would be best deferred down to md_apply_fix, but then we
don't have code to handle the microMIPS case there. Iain's been doing
some work in this area recently -- I'm not sure how far he's got though.
> I.e. something like:
>
> case BFD_RELOC_16_PCREL_S2:
> {
> int shift;
>
> shift = mips_opts.micromips ? 1 : 2;
> if ((address_expr->X_add_number & ((1 << shift) - 1)) != 0)
> as_bad (_("branch to misaligned address (0x%lx)"),
> (unsigned long) address_expr->X_add_number);
> if (!mips_relax_branch && !mips_opts.micromips)
> {
> if ((address_expr->X_add_number + (1 << (shift + 15)))
> & ~((1 << (shift + 16)) - 1))
> as_bad (_("branch address range overflow (0x%lx)"),
> (unsigned long) address_expr->X_add_number);
> ip->insn_opcode |= ((address_expr->X_add_number >> shift)
> & 0xffff);
> ip->complete_p = 1;
> }
> else
> ip->complete_p = 0; /* Redundant after next patch. */
> }
>
> with the new micromips condition forcing the usual branch reloc
> to be created. It looks like that doesn't work for:
>
> .set micromips
> beq $4,$5,0x4000
>
> though, for reasons I've not investigated. It seems there are other
> problems with these kinds of instruction too, even without the change
> above. E.g.:
>
> .set micromips
> bc1tl 0x4000
>
> triggers an assertion failure while:
>
> .set micromips
> beqz $4,0x4000
>
> causes a segfault. So let's leave that for another day and just stick
> to the cases you're fixing.
Presumably because you've got a mixture of macros and real instructions
here -- depending on which path is taken different results come out. Oh
well...
> I changed the patch to explicitly set complete_p to 1, for the benefit
> of the patch I'm about to post. Applied with that change, thanks.
OK, thanks.
Maciej