[PATCH 5/6] Add Visium support to gas

[Alan Modra]

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