This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH 5/6] Add Visium support to gas
- From: Alan Modra <amodra at gmail dot com>
- To: Eric Botcazou <ebotcazou at adacore dot com>
- Cc: binutils at sourceware dot org, Eric Botcazou <ebotcazou at gcc dot gnu dot org>
- Date: Fri, 5 Dec 2014 12:55:51 +1030
- Subject: Re: [PATCH 5/6] Add Visium support to gas
- Authentication-results: sourceware.org; auth=none
- References: <cover dot 1417646850 dot git dot ebotcazou at gcc dot gnu dot org> <fee07d8c32de19e9586f02a00a656e3628d5f68f dot 1417646850 dot git dot ebotcazou at gcc dot gnu dot org>
On Thu, Dec 04, 2014 at 07:08:33AM -0500, Eric Botcazou wrote:
> +void
> +md_apply_fix (fixS * fixP, valueT * value, segT segment)
[snip]
> + case BFD_RELOC_VISIUM_HI16:
> + case BFD_RELOC_VISIUM_HI16_PCREL:
> + if (!fixP->fx_addsy)
> + {
> + insn = (insn & 0xffff0000) | ((val >> 16) & 0x0000ffff);
> + }
> + else
> + {
> + /* FIXME: Need comment explaining why we do this. */
> + insn = (insn & 0xffff0000);
> + }
> + break;
Heh. I'd like an explanation too. :) If the clearing is necessary
it indicates to me that you have a bug somewhere in md_assemble
setting the field non-zero..
BTW, since your target is rela, you don't need to write to the field
if you're emitting a reloc (which will happen if fx_done is clear on
exit from md_apply_fix). It's cleaner to leave the field zero rather
than writing in the addend.
Also, I don't see any check on fx_subsy. You should emit an error if
fx_subsy is present and not handled.
> + /* Are we finished with this relocation now? */
> + if (fixP->fx_addsy == 0 && !fixP->fx_pcrel)
> + fixP->fx_done = 1;
That !fixP->fx_pcrel lools suspicious. Why punt all pcrel to the
linker?
--
Alan Modra
Australia Development Lab, IBM