| Message ID | 1310527849-784-2-git-send-email-tsnsaito@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jul 13, 2011 at 6:30 AM, Tsuneo Saito <tsnsaito@gmail.com> wrote: > This patch implements sparcv9 ldfa/lddfa/ldqfa instructions > with non block-load ASIs. > > Signed-off-by: Tsuneo Saito <tsnsaito@gmail.com> > --- > target-sparc/op_helper.c | 16 +++++++++++----- > 1 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c > index fd0cfbd..a75ac4f 100644 > --- a/target-sparc/op_helper.c > +++ b/target-sparc/op_helper.c > @@ -3331,7 +3331,7 @@ void helper_ldda_asi(target_ulong addr, int asi, int rd) > void helper_ldf_asi(target_ulong addr, int asi, int size, int rd) > { > unsigned int i; > - target_ulong val; > + CPU_DoubleU u; > > helper_check_align(addr, 3); > addr = asi_address_mask(env, asi, addr); > @@ -3371,17 +3371,23 @@ void helper_ldf_asi(target_ulong addr, int asi, int size, int rd) > break; > } > > - val = helper_ld_asi(addr, asi, size, 0); > switch(size) { > default: > case 4: > - *((uint32_t *)&env->fpr[rd]) = val; > + *((uint32_t *)&env->fpr[rd]) = helper_ld_asi(addr, asi, size, 0); > break; > case 8: > - *((int64_t *)&DT0) = val; > + u.ll = helper_ld_asi(addr, asi, size, 0); > + *((uint32_t *)&env->fpr[rd++]) = u.l.upper; > + *((uint32_t *)&env->fpr[rd++]) = u.l.lower; > break; > case 16: > - // XXX > + u.ll = helper_ld_asi(addr, asi, 8, 0); > + *((uint32_t *)&env->fpr[rd++]) = u.l.upper; > + *((uint32_t *)&env->fpr[rd++]) = u.l.lower; > + u.ll = helper_ld_asi(addr + 8, asi, 8, 0); In general the patch is an improvement, however the ASIs are passed as is to helper_ld_asi() and there the block ASIs would trigger unassigned access faults. So you should perform some arithmetic with the ASI numbers to make them match non-block ASIs, for example asi & ~0x6. This only works in the specific block ASIs, so I'd move this inside the switch block for cases 0x16, 0x17, 0x1e, 0x1f etc. By the way, aren't those the ASIs in question? The manual (UltraSPARC Architecture 2007) is a bit confusing. > + *((uint32_t *)&env->fpr[rd++]) = u.l.upper; > + *((uint32_t *)&env->fpr[rd++]) = u.l.lower; > break; > } > } > -- > 1.7.5.4 > > >
On Wed, Jul 13, 2011 at 6:27 PM, Blue Swirl <blauwirbel@gmail.com> wrote: > On Wed, Jul 13, 2011 at 6:30 AM, Tsuneo Saito <tsnsaito@gmail.com> wrote: >> This patch implements sparcv9 ldfa/lddfa/ldqfa instructions >> with non block-load ASIs. >> >> Signed-off-by: Tsuneo Saito <tsnsaito@gmail.com> >> --- >> target-sparc/op_helper.c | 16 +++++++++++----- >> 1 files changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c >> index fd0cfbd..a75ac4f 100644 >> --- a/target-sparc/op_helper.c >> +++ b/target-sparc/op_helper.c >> @@ -3331,7 +3331,7 @@ void helper_ldda_asi(target_ulong addr, int asi, int rd) >> void helper_ldf_asi(target_ulong addr, int asi, int size, int rd) >> { >> unsigned int i; >> - target_ulong val; >> + CPU_DoubleU u; >> >> helper_check_align(addr, 3); >> addr = asi_address_mask(env, asi, addr); >> @@ -3371,17 +3371,23 @@ void helper_ldf_asi(target_ulong addr, int asi, int size, int rd) >> break; >> } >> >> - val = helper_ld_asi(addr, asi, size, 0); >> switch(size) { >> default: >> case 4: >> - *((uint32_t *)&env->fpr[rd]) = val; >> + *((uint32_t *)&env->fpr[rd]) = helper_ld_asi(addr, asi, size, 0); >> break; >> case 8: >> - *((int64_t *)&DT0) = val; >> + u.ll = helper_ld_asi(addr, asi, size, 0); >> + *((uint32_t *)&env->fpr[rd++]) = u.l.upper; >> + *((uint32_t *)&env->fpr[rd++]) = u.l.lower; >> break; >> case 16: >> - // XXX >> + u.ll = helper_ld_asi(addr, asi, 8, 0); >> + *((uint32_t *)&env->fpr[rd++]) = u.l.upper; >> + *((uint32_t *)&env->fpr[rd++]) = u.l.lower; >> + u.ll = helper_ld_asi(addr + 8, asi, 8, 0); > > In general the patch is an improvement, however the ASIs are passed as > is to helper_ld_asi() and there the block ASIs would trigger > unassigned access faults. You mean the not [yet] implemented block ASIs? The implemented ones return earlier and for the not implemented ones unassigned access fault appears to be a right thing to do. Do you have an example where we would have an unassigned access fault for an implemented ASI ? >So you should perform some arithmetic with > the ASI numbers to make them match non-block ASIs, for example asi & > ~0x6. This only works in the specific block ASIs, so I'd move this > inside the switch block for cases 0x16, 0x17, 0x1e, 0x1f etc. By the > way, aren't those the ASIs in question? The manual (UltraSPARC > Architecture 2007) is a bit confusing. > >> + *((uint32_t *)&env->fpr[rd++]) = u.l.upper; >> + *((uint32_t *)&env->fpr[rd++]) = u.l.lower; >> break; >> } >> } >> -- >> 1.7.5.4 >>
On Wed, Jul 13, 2011 at 9:02 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote: > On Wed, Jul 13, 2011 at 6:27 PM, Blue Swirl <blauwirbel@gmail.com> wrote: >> On Wed, Jul 13, 2011 at 6:30 AM, Tsuneo Saito <tsnsaito@gmail.com> wrote: >>> This patch implements sparcv9 ldfa/lddfa/ldqfa instructions >>> with non block-load ASIs. >>> >>> Signed-off-by: Tsuneo Saito <tsnsaito@gmail.com> >>> --- >>> target-sparc/op_helper.c | 16 +++++++++++----- >>> 1 files changed, 11 insertions(+), 5 deletions(-) >>> >>> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c >>> index fd0cfbd..a75ac4f 100644 >>> --- a/target-sparc/op_helper.c >>> +++ b/target-sparc/op_helper.c >>> @@ -3331,7 +3331,7 @@ void helper_ldda_asi(target_ulong addr, int asi, int rd) >>> void helper_ldf_asi(target_ulong addr, int asi, int size, int rd) >>> { >>> unsigned int i; >>> - target_ulong val; >>> + CPU_DoubleU u; >>> >>> helper_check_align(addr, 3); >>> addr = asi_address_mask(env, asi, addr); >>> @@ -3371,17 +3371,23 @@ void helper_ldf_asi(target_ulong addr, int asi, int size, int rd) >>> break; >>> } >>> >>> - val = helper_ld_asi(addr, asi, size, 0); >>> switch(size) { >>> default: >>> case 4: >>> - *((uint32_t *)&env->fpr[rd]) = val; >>> + *((uint32_t *)&env->fpr[rd]) = helper_ld_asi(addr, asi, size, 0); >>> break; >>> case 8: >>> - *((int64_t *)&DT0) = val; >>> + u.ll = helper_ld_asi(addr, asi, size, 0); >>> + *((uint32_t *)&env->fpr[rd++]) = u.l.upper; >>> + *((uint32_t *)&env->fpr[rd++]) = u.l.lower; >>> break; >>> case 16: >>> - // XXX >>> + u.ll = helper_ld_asi(addr, asi, 8, 0); >>> + *((uint32_t *)&env->fpr[rd++]) = u.l.upper; >>> + *((uint32_t *)&env->fpr[rd++]) = u.l.lower; >>> + u.ll = helper_ld_asi(addr + 8, asi, 8, 0); >> >> In general the patch is an improvement, however the ASIs are passed as >> is to helper_ld_asi() and there the block ASIs would trigger >> unassigned access faults. > > You mean the not [yet] implemented block ASIs? The implemented ones > return earlier > and for the not implemented ones unassigned access fault appears to be > a right thing to do. > Do you have an example where we would have an unassigned access fault > for an implemented ASI ? This patch would implement the unimplemented ones by calling helper_ld_asi(), but the ASIs do not match. Instead of 0x1f (as if user little endian secondary address space block ASI), 0x19 (corresponding non-block ASI) should be used. Also helper_ld_asi() should not be changed to accept block ASIs, because then they would be accepted also for non-block accesses. >>So you should perform some arithmetic with >> the ASI numbers to make them match non-block ASIs, for example asi & >> ~0x6. This only works in the specific block ASIs, so I'd move this >> inside the switch block for cases 0x16, 0x17, 0x1e, 0x1f etc. By the >> way, aren't those the ASIs in question? The manual (UltraSPARC >> Architecture 2007) is a bit confusing. >> >>> + *((uint32_t *)&env->fpr[rd++]) = u.l.upper; >>> + *((uint32_t *)&env->fpr[rd++]) = u.l.lower; >>> break; >>> } >>> } >>> -- >>> 1.7.5.4 >>> > > > > -- > Regards, > Artyom Tarasenko > > solaris/sparc under qemu blog: http://tyom.blogspot.com/ >
Hi, At Wed, 13 Jul 2011 21:19:16 +0300, Blue Swirl wrote: > On Wed, Jul 13, 2011 at 9:02 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote: > > On Wed, Jul 13, 2011 at 6:27 PM, Blue Swirl <blauwirbel@gmail.com> wrote: > >> On Wed, Jul 13, 2011 at 6:30 AM, Tsuneo Saito <tsnsaito@gmail.com> wrote: > >>So you should perform some arithmetic with > >> the ASI numbers to make them match non-block ASIs, for example asi & > >> ~0x6. This only works in the specific block ASIs, so I'd move this > >> inside the switch block for cases 0x16, 0x17, 0x1e, 0x1f etc. By the > >> way, aren't those the ASIs in question? The manual (UltraSPARC > >> Architecture 2007) is a bit confusing. Thanks for the review! I didn't know these ASIs (0x16, 0x17, 0x1e, 0x1f) as I was looking at the JPS 1.0.4 specification. I'll add codes for these ASIs. ---- Tsuneo Saito <tsnsaito@gmail.com>
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c index fd0cfbd..a75ac4f 100644 --- a/target-sparc/op_helper.c +++ b/target-sparc/op_helper.c @@ -3331,7 +3331,7 @@ void helper_ldda_asi(target_ulong addr, int asi, int rd) void helper_ldf_asi(target_ulong addr, int asi, int size, int rd) { unsigned int i; - target_ulong val; + CPU_DoubleU u; helper_check_align(addr, 3); addr = asi_address_mask(env, asi, addr); @@ -3371,17 +3371,23 @@ void helper_ldf_asi(target_ulong addr, int asi, int size, int rd) break; } - val = helper_ld_asi(addr, asi, size, 0); switch(size) { default: case 4: - *((uint32_t *)&env->fpr[rd]) = val; + *((uint32_t *)&env->fpr[rd]) = helper_ld_asi(addr, asi, size, 0); break; case 8: - *((int64_t *)&DT0) = val; + u.ll = helper_ld_asi(addr, asi, size, 0); + *((uint32_t *)&env->fpr[rd++]) = u.l.upper; + *((uint32_t *)&env->fpr[rd++]) = u.l.lower; break; case 16: - // XXX + u.ll = helper_ld_asi(addr, asi, 8, 0); + *((uint32_t *)&env->fpr[rd++]) = u.l.upper; + *((uint32_t *)&env->fpr[rd++]) = u.l.lower; + u.ll = helper_ld_asi(addr + 8, asi, 8, 0); + *((uint32_t *)&env->fpr[rd++]) = u.l.upper; + *((uint32_t *)&env->fpr[rd++]) = u.l.lower; break; } }
This patch implements sparcv9 ldfa/lddfa/ldqfa instructions with non block-load ASIs. Signed-off-by: Tsuneo Saito <tsnsaito@gmail.com> --- target-sparc/op_helper.c | 16 +++++++++++----- 1 files changed, 11 insertions(+), 5 deletions(-)