Message ID | 1380242934-20953-27-git-send-email-agraf@suse.de |
---|---|
State | New |
Headers | show |
Hello all, On 09/27/2013 02:48 AM, Alexander Graf wrote: > This patch adds emulation support for the adr instruction. > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > target-arm/translate-a64.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index bc91324..00eda0f 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -943,6 +943,27 @@ static void handle_insg(DisasContext *s, uint32_t insn) > simd_st(cpu_reg(rn), freg_offs_d + idx, size); > } > > +/* PC relative address calculation */ > +static void handle_adr(DisasContext *s, uint32_t insn) > +{ > + int reg = get_reg(insn); > + int is_page = get_bits(insn, 31, 1); > + uint64_t imm; > + uint64_t base; > + > + imm = get_sbits(insn, 5, 19) << 2; > + imm |= get_bits(insn, 29, 2); > + > + base = s->pc - 4; > + if (is_page) { > + /* ADRP (page based) */ > + base &= ~0xFFFULL; > + imm <<= 12; > + } > + > + tcg_gen_movi_i64(cpu_reg(reg), base + imm); > +} > + does this work with negative values? The spec says to SignExtend: if page then imm = SignExtend(immhi:immlo:Zeros(12), 64); else imm = SignExtend(immhi:immlo, 64); /*...*/ maybe Michael you know if this is an issue in practice? If I want to get a negative PC relative offset, how does this work? Claudio > /* SIMD ORR */ > static void handle_simdorr(DisasContext *s, uint32_t insn) > { > @@ -1365,6 +1386,9 @@ void disas_a64_insn(CPUARMState *env, DisasContext *s) > unallocated_encoding(s); > } > break; > + case 0x10: > + handle_adr(s, insn); > + break; > default: > unallocated_encoding(s); > break; >
On 11/19/2013 06:17 PM, Claudio Fontana wrote: > Hello all, > > On 09/27/2013 02:48 AM, Alexander Graf wrote: >> This patch adds emulation support for the adr instruction. >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> target-arm/translate-a64.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c >> index bc91324..00eda0f 100644 >> --- a/target-arm/translate-a64.c >> +++ b/target-arm/translate-a64.c >> @@ -943,6 +943,27 @@ static void handle_insg(DisasContext *s, uint32_t insn) >> simd_st(cpu_reg(rn), freg_offs_d + idx, size); >> } >> >> +/* PC relative address calculation */ >> +static void handle_adr(DisasContext *s, uint32_t insn) >> +{ >> + int reg = get_reg(insn); >> + int is_page = get_bits(insn, 31, 1); >> + uint64_t imm; >> + uint64_t base; >> + >> + imm = get_sbits(insn, 5, 19) << 2; >> + imm |= get_bits(insn, 29, 2); >> + >> + base = s->pc - 4; >> + if (is_page) { >> + /* ADRP (page based) */ >> + base &= ~0xFFFULL; >> + imm <<= 12; >> + } >> + >> + tcg_gen_movi_i64(cpu_reg(reg), base + imm); >> +} >> + > > does this work with negative values? > The spec says to SignExtend: > > if page then > imm = SignExtend(immhi:immlo:Zeros(12), 64); > else > imm = SignExtend(immhi:immlo, 64); > > /*...*/ > > maybe Michael you know if this is an issue in practice? > If I want to get a negative PC relative offset, how does this work? > > Claudio This is a totally untested idea of what I would think/roughly implement: static void disas_pc_rel_adr(DisasContext *s, uint32_t insn) { /* * 31 30 29 28 27 26 25 24 23 5 4 0 * op immlo 1 0 0 0 0 immhi Rd */ unsigned int page, imm, rd, len; /* op -> page, immhi:immlo -> imm */ uint64_t base; sint64_t offset; /* SignExtend(imm) -> offset */ page = insn & (1 << 31) ? 1 : 0; imm = extract32(insn, 29, 2) + extract32(insn, 5, 19) << 2; rd = extract32(insn, 0, 5); base = s->pc - 4; len = 19 + 2; /* immhi:immlo */ offset = imm; if (page) { /* ADRP (page based) */ base &= ~0xfff; len += 12; /* immhi:immlo:Zeros(12) */ offset <<= 12; /* apply Zeros */ } offset = (offset << (64 - len)) >> (64 - len); /* sign extend */ tcg_gen_movi_i64(cpu_reg(reg), base + offset); } But maybe I am completely off and the original code is perfectly fine..? C. > > >> /* SIMD ORR */ >> static void handle_simdorr(DisasContext *s, uint32_t insn) >> { >> @@ -1365,6 +1386,9 @@ void disas_a64_insn(CPUARMState *env, DisasContext *s) >> unallocated_encoding(s); >> } >> break; >> + case 0x10: >> + handle_adr(s, insn); >> + break; >> default: >> unallocated_encoding(s); >> break; >> >
On 19 November 2013 17:52, Claudio Fontana <claudio.fontana@linaro.org> wrote: > static void disas_pc_rel_adr(DisasContext *s, uint32_t insn) > { > /* > * 31 30 29 28 27 26 25 24 23 5 4 0 > * op immlo 1 0 0 0 0 immhi Rd > */ > unsigned int page, imm, rd, len; /* op -> page, immhi:immlo -> imm */ > uint64_t base; > sint64_t offset; /* SignExtend(imm) -> offset */ > > page = insn & (1 << 31) ? 1 : 0; > imm = extract32(insn, 29, 2) + extract32(insn, 5, 19) << 2; > rd = extract32(insn, 0, 5); Claiming you want sign extension and not using sextract32() is a bit odd. > > base = s->pc - 4; > len = 19 + 2; /* immhi:immlo */ > offset = imm; > > if (page) { > /* ADRP (page based) */ > base &= ~0xfff; > len += 12; /* immhi:immlo:Zeros(12) */ > offset <<= 12; /* apply Zeros */ > } > > offset = (offset << (64 - len)) >> (64 - len); /* sign extend */ Don't manually sign extend, please. > tcg_gen_movi_i64(cpu_reg(reg), base + offset); > } thanks -- PMM
On 19 November 2013 18:03, Peter Maydell <peter.maydell@linaro.org> wrote: > On 19 November 2013 17:52, Claudio Fontana <claudio.fontana@linaro.org> wrote: >> static void disas_pc_rel_adr(DisasContext *s, uint32_t insn) >> { >> /* >> * 31 30 29 28 27 26 25 24 23 5 4 0 >> * op immlo 1 0 0 0 0 immhi Rd >> */ >> unsigned int page, imm, rd, len; /* op -> page, immhi:immlo -> imm */ >> uint64_t base; >> sint64_t offset; /* SignExtend(imm) -> offset */ >> >> page = insn & (1 << 31) ? 1 : 0; >> imm = extract32(insn, 29, 2) + extract32(insn, 5, 19) << 2; >> rd = extract32(insn, 0, 5); > > Claiming you want sign extension and not using sextract32() > is a bit odd. ...to be a bit more specific, you can get the immediate into a signed 64 bit variable like this: int64_t imm = (sextract32(insn, 5, 19) << 2) | extract32(insn, 29, 2); if (page) { imm <<= 12; } which is exactly what Alex's original patch does, except it's not using the standard sextract/extract functions (probably because they weren't in master at the point he wrote it). thanks -- PMM
Hi, On Tue, 19 Nov 2013, Claudio Fontana wrote: > > + uint64_t imm; > > + uint64_t base; > > + > > + imm = get_sbits(insn, 5, 19) << 2; > > + imm |= get_bits(insn, 29, 2); > > does this work with negative values? Yes. get_sbits returns a sign extended (32bit) int, the shift doesn't change that, the conversion to uint64_t first sign extends to 64bit and then converts to unsigned (conceptually). From then on it's an unsigned value (with high bits set when input was negative), but two complement arithmetic makes that irrelevant. Ciao, Michael.
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c index bc91324..00eda0f 100644 --- a/target-arm/translate-a64.c +++ b/target-arm/translate-a64.c @@ -943,6 +943,27 @@ static void handle_insg(DisasContext *s, uint32_t insn) simd_st(cpu_reg(rn), freg_offs_d + idx, size); } +/* PC relative address calculation */ +static void handle_adr(DisasContext *s, uint32_t insn) +{ + int reg = get_reg(insn); + int is_page = get_bits(insn, 31, 1); + uint64_t imm; + uint64_t base; + + imm = get_sbits(insn, 5, 19) << 2; + imm |= get_bits(insn, 29, 2); + + base = s->pc - 4; + if (is_page) { + /* ADRP (page based) */ + base &= ~0xFFFULL; + imm <<= 12; + } + + tcg_gen_movi_i64(cpu_reg(reg), base + imm); +} + /* SIMD ORR */ static void handle_simdorr(DisasContext *s, uint32_t insn) { @@ -1365,6 +1386,9 @@ void disas_a64_insn(CPUARMState *env, DisasContext *s) unallocated_encoding(s); } break; + case 0x10: + handle_adr(s, insn); + break; default: unallocated_encoding(s); break;
This patch adds emulation support for the adr instruction. Signed-off-by: Alexander Graf <agraf@suse.de> --- target-arm/translate-a64.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)