Re: [GOLD] Edit PowerPC64 ELFv2 function entry code

[Alan Modra <amodra at gmail dot com>]

On Tue, Dec 08, 2015 at 02:36:12PM -0800, Cary Coutant wrote:
> > +                 Insn* iview = reinterpret_cast<Insn*>(view - 2 * big_endian);
> > +                 Insn insn1 = elfcpp::Swap<32, big_endian>::readval(iview - 1);
> > +                 Insn insn2 = elfcpp::Swap<32, big_endian>::readval(iview);
> 
> I'm afraid I don't understand this logic. As I read it, you're looking
> at view for little-endian, and at (view - 2) for big-endian. If that's
> the case, I think you should be using Swap_unaligned here, but I don't
> see why you offset the view by -2 for big-endian.

The reloc in question operates on a two-byte field of the four-byte
instruction.   When the insn is little-endian the field and the insn
start at the same address.  Not so for big-endian.

> As a matter of style, since big_endian is a bool, I'd prefer to see
> this written in conditional form, e.g., (view - (big_endian ? 2 : 0)).
> 
> > +             r_type = elfcpp::R_POWERPC_ADDR16_HA;
> > +             addend -= 2 * big_endian;
> 
> Likewise, "if (big_endian) addend -= 2;". Easier (for me, at least), to read.

I use this style all over powerpc.cc::Relocate::relocate.  The main
reason being that it is a little more concise and
	      Insn* iview = reinterpret_cast<Insn*>(view - 2 * big_endian);
is getting close to the 80 char line limit.  I'll fix it after this
patch goes in to use

    // Offset from start of insn to d-field reloc.
    const int d_offset = big_endian ? 2 : 0;

-- 
Alan Modra
Australia Development Lab, IBM