| Message ID | 1362158507-19310-4-git-send-email-chouteau@adacore.com |
|---|---|
| State | New |
| Headers | show |
> From GDB Remote Serial Protocol doc: > > "The bytes with the register are transmitted in target byte order." > /* Aliases for Q regs. */ > nregs += 16; > if (reg < nregs) { > > - stfq_le_p(buf, env->vfp.regs[(reg - 32) * 2]); > - stfq_le_p(buf + 8, env->vfp.regs[(reg - 32) * 2 + 1]); > + stfq_p(buf, env->vfp.regs[(reg - 32) * 2]); > + stfq_p(buf + 8, env->vfp.regs[(reg - 32) * 2 + 1]); This is wrong. You're still using little-endian ordering of words. Paul
On 03/01/2013 09:51 PM, Paul Brook wrote: >> From GDB Remote Serial Protocol doc: >> >> "The bytes with the register are transmitted in target byte order." > >> /* Aliases for Q regs. */ >> nregs += 16; >> if (reg < nregs) { >> >> - stfq_le_p(buf, env->vfp.regs[(reg - 32) * 2]); >> - stfq_le_p(buf + 8, env->vfp.regs[(reg - 32) * 2 + 1]); >> + stfq_p(buf, env->vfp.regs[(reg - 32) * 2]); >> + stfq_p(buf + 8, env->vfp.regs[(reg - 32) * 2 + 1]); > > This is wrong. You're still using little-endian ordering of words. > Can you explain a little bit further? If I'm in big-endian mode, stfq_p() will be stfq_be_p(), right? Thanks,
> >> "The bytes with the register are transmitted in target byte order." > >> > >> /* Aliases for Q regs. */ > >> nregs += 16; > >> if (reg < nregs) { > >> > >> - stfq_le_p(buf, env->vfp.regs[(reg - 32) * 2]); > >> - stfq_le_p(buf + 8, env->vfp.regs[(reg - 32) * 2 + 1]); > >> + stfq_p(buf, env->vfp.regs[(reg - 32) * 2]); > >> + stfq_p(buf + 8, env->vfp.regs[(reg - 32) * 2 + 1]); > > > > This is wrong. You're still using little-endian ordering of words. > > Can you explain a little bit further? If I'm in big-endian mode, stfq_p() > will be stfq_be_p(), right? Because we're actually storing two halves of a 128-bit value. You still store the least significant half first. Paul
On 03/04/2013 02:30 PM, Paul Brook wrote: >>>> "The bytes with the register are transmitted in target byte order." >>>> >>>> /* Aliases for Q regs. */ >>>> nregs += 16; >>>> if (reg < nregs) { >>>> >>>> - stfq_le_p(buf, env->vfp.regs[(reg - 32) * 2]); >>>> - stfq_le_p(buf + 8, env->vfp.regs[(reg - 32) * 2 + 1]); >>>> + stfq_p(buf, env->vfp.regs[(reg - 32) * 2]); >>>> + stfq_p(buf + 8, env->vfp.regs[(reg - 32) * 2 + 1]); >>> >>> This is wrong. You're still using little-endian ordering of words. >> >> Can you explain a little bit further? If I'm in big-endian mode, stfq_p() >> will be stfq_be_p(), right? > > Because we're actually storing two halves of a 128-bit value. You still > store the least significant half first. > Right, I'm sorry I didn't see you comment was only about the Q registers. What would be the solution then? #ifdef TARGET_WORDS_BIGENDIAN stfq_p(buf, env->vfp.regs[(reg - 32) * 2 + 1]); stfq_p(buf + 8, env->vfp.regs[(reg - 32) * 2]); #else stfq_p(buf, env->vfp.regs[(reg - 32) * 2]); stfq_p(buf + 8, env->vfp.regs[(reg - 32) * 2 + 1]); #endif /* TARGET_WORDS_BIGENDIAN */ Regards,
On 03/05/2013 12:34 AM, Paul Brook wrote: >>> Because we're actually storing two halves of a 128-bit value. You still >>> store the least significant half first. >> >> Right, I'm sorry I didn't see you comment was only about the Q registers. >> What would be the solution then? >> >> #ifdef TARGET_WORDS_BIGENDIAN >> stfq_p(buf, env->vfp.regs[(reg - 32) * 2 + 1]); >> stfq_p(buf + 8, env->vfp.regs[(reg - 32) * 2]); >> #else >> stfq_p(buf, env->vfp.regs[(reg - 32) * 2]); >> stfq_p(buf + 8, env->vfp.regs[(reg - 32) * 2 + 1]); >> #endif /* TARGET_WORDS_BIGENDIAN */ > > Yes, something like that. > Alright, I'll do that for V2. Thanks,
diff --git a/target-arm/helper.c b/target-arm/helper.c index e97e1a5..75ee0dc 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -16,18 +16,17 @@ static int vfp_gdb_get_reg(CPUARMState *env, uint8_t *buf, int reg) { int nregs; - /* VFP data registers are always little-endian. */ nregs = arm_feature(env, ARM_FEATURE_VFP3) ? 32 : 16; if (reg < nregs) { - stfq_le_p(buf, env->vfp.regs[reg]); + stfq_p(buf, env->vfp.regs[reg]); return 8; } if (arm_feature(env, ARM_FEATURE_NEON)) { /* Aliases for Q regs. */ nregs += 16; if (reg < nregs) { - stfq_le_p(buf, env->vfp.regs[(reg - 32) * 2]); - stfq_le_p(buf + 8, env->vfp.regs[(reg - 32) * 2 + 1]); + stfq_p(buf, env->vfp.regs[(reg - 32) * 2]); + stfq_p(buf + 8, env->vfp.regs[(reg - 32) * 2 + 1]); return 16; } } @@ -45,14 +44,14 @@ static int vfp_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg) nregs = arm_feature(env, ARM_FEATURE_VFP3) ? 32 : 16; if (reg < nregs) { - env->vfp.regs[reg] = ldfq_le_p(buf); + env->vfp.regs[reg] = ldfq_p(buf); return 8; } if (arm_feature(env, ARM_FEATURE_NEON)) { nregs += 16; if (reg < nregs) { - env->vfp.regs[(reg - 32) * 2] = ldfq_le_p(buf); - env->vfp.regs[(reg - 32) * 2 + 1] = ldfq_le_p(buf + 8); + env->vfp.regs[(reg - 32) * 2] = ldfq_p(buf); + env->vfp.regs[(reg - 32) * 2 + 1] = ldfq_p(buf + 8); return 16; } }
From GDB Remote Serial Protocol doc: "The bytes with the register are transmitted in target byte order." Signed-off-by: Fabien Chouteau <chouteau@adacore.com> --- target-arm/helper.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)