Re: [PATCH 1/2] aarch64: extend decode_adrp to decode immediate offset

[Kyle McMartin <kmcmarti@redhat.com>]

On Tue, Jun 03, 2014 at 09:22:28AM +0100, Marcus Shawcroft wrote:
> Hi,
> 
> On 3 June 2014 06:01, Kyle McMartin <kmcmarti@redhat.com> wrote:
> 
> > @@ -264,16 +264,28 @@ decode_add_sub_imm (CORE_ADDR addr, uint32_t insn, unsigned *rd, unsigned *rn,
> >     Return 1 if the opcodes matches and is decoded, otherwise 0.  */
> >
> >  static int
> > -decode_adrp (CORE_ADDR addr, uint32_t insn, unsigned *rd)
> > +decode_adrp (CORE_ADDR addr, uint32_t insn, int *page, unsigned *rd,
> > +            int64_t *imm)
> 
> Given that this now decodes both adrp and adr the function name seems
> inappropriate, how about following the convention used in the other
> decode_ functions and changing to something like decode_adrp_adr ()..
> ?
> 

Sure, sounds good.

> Returning both 'page' and 'imm' isn't necessary and I don't think it
> makes sense given that logically both adr and adrp are used to
> construct an address.  Looking at this patch and the following patch I
> think you can achieve the same functionality by having the decodes of
> both adrp and adr return a value in 'imm'.  In the case of an adrp
> decode just returning: imm = page << 12  ....
> 

Hmm, I was trying to avoid doing the actual computation inside it. I'm
happy one way or another though.

--Kyle

> Cheers
> /Marcus