| Message ID | 1305671572-5899-22-git-send-email-jcmvbkbc@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 05/17/2011 03:32 PM, Max Filippov wrote: > See ISA, 4.4.4 for details. > > Correct (aligned as per ISA) address for unaligned access is generated > in case this option is not enabled. > > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> > --- > target-xtensa/translate.c | 33 +++++++++++++++++++++++++++++++-- > 1 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c > index 592072a..6e66f3f 100644 > --- a/target-xtensa/translate.c > +++ b/target-xtensa/translate.c > @@ -177,6 +177,16 @@ static void gen_exception_cause(DisasContext *dc, uint32_t cause) > tcg_temp_free(_cause); > } > > +static void gen_exception_cause_vaddr(DisasContext *dc, uint32_t cause, > + TCGv_i32 vaddr) > +{ > + TCGv_i32 _pc = tcg_const_i32(dc->pc); > + TCGv_i32 _cause = tcg_const_i32(cause); > + gen_helper_exception_cause_vaddr(_pc, _cause, vaddr); > + tcg_temp_free(_pc); > + tcg_temp_free(_cause); > +} > + > static void gen_check_privilege(DisasContext *dc) > { > if (dc->mem_idx) { > @@ -349,6 +359,20 @@ static void gen_wsr(DisasContext *dc, uint32_t sr, TCGv_i32 s) > } > } > > +static void gen_load_store_alignment(DisasContext *dc, int shift, TCGv_i32 addr) > +{ > + TCGv_i32 tmp = tcg_temp_local_new_i32(); > + tcg_gen_mov_i32(tmp, addr); > + tcg_gen_andi_i32(addr, addr, ~0 << shift); > + if (option_enabled(dc, XTENSA_OPTION_UNALIGNED_EXCEPTION)) { > + int label = gen_new_label(); > + tcg_gen_brcond_i32(TCG_COND_EQ, addr, tmp, label); > + gen_exception_cause_vaddr(dc, LOAD_STORE_ALIGNMENT_CAUSE, tmp); > + gen_set_label(label); > + } > + tcg_temp_free(tmp); > +} This is not the correct method for this. Set ALIGNED_ONLY before defining the softmmu_templates. Define do_unaligned_access to raise the exception. See e.g. target-sparc/op_helper.c. r~
> > +static void gen_load_store_alignment(DisasContext *dc, int shift, TCGv_i32 addr) > > +{ > > + TCGv_i32 tmp = tcg_temp_local_new_i32(); > > + tcg_gen_mov_i32(tmp, addr); > > + tcg_gen_andi_i32(addr, addr, ~0 << shift); > > + if (option_enabled(dc, XTENSA_OPTION_UNALIGNED_EXCEPTION)) { > > + int label = gen_new_label(); > > + tcg_gen_brcond_i32(TCG_COND_EQ, addr, tmp, label); > > + gen_exception_cause_vaddr(dc, LOAD_STORE_ALIGNMENT_CAUSE, tmp); > > + gen_set_label(label); > > + } > > + tcg_temp_free(tmp); > > +} > > This is not the correct method for this. Set ALIGNED_ONLY before > defining the softmmu_templates. Define do_unaligned_access to raise > the exception. See e.g. target-sparc/op_helper.c. There are three alignment possibilities for xtensa: no unaligned exception, unaligned exception and hardware alignment. In the first case unaligned access silently goes to aligned address. It looks like it cannot be done via do_unaligned_access, can it? In the third case most unaligned accesses are handled transparently by the hardware. But e.g. unaligned access by multiprocessor synchronization instructions still cause alignment exception. Do I need to implement a different alignment checking mechanism for those unhandled cases? Thanks. -- Max
On 05/22/2011 05:10 AM, Max Filippov wrote: > There are three alignment possibilities for xtensa: no unaligned > exception, unaligned exception and hardware alignment. In the first > case unaligned access silently goes to aligned address. It looks like > it cannot be done via do_unaligned_access, can it? In the third case > most unaligned accesses are handled transparently by the hardware. > But e.g. unaligned access by multiprocessor synchronization > instructions still cause alignment exception. Do I need to implement > a different alignment checking mechanism for those unhandled cases? Case (1), silently going to an aligned address, should be handled inside the translator by masking the address before the load. See the ARM and Alpha targets for examples. Case (2) and (3) are both handled by using ALIGNED_ONLY. All you need to do to handle (3) is *not* throw an exception from the do_unaligned_access function. See how the code is structured inside softmmu_template.h. As for sync insns... You may need to handle it all out-of-line and check for alignment there. r~
> > There are three alignment possibilities for xtensa: no unaligned > > exception, unaligned exception and hardware alignment. In the first > > case unaligned access silently goes to aligned address. It looks like > > it cannot be done via do_unaligned_access, can it? In the third case > > most unaligned accesses are handled transparently by the hardware. > > But e.g. unaligned access by multiprocessor synchronization > > instructions still cause alignment exception. Do I need to implement > > a different alignment checking mechanism for those unhandled cases? > > Case (1), silently going to an aligned address, should be handled > inside the translator by masking the address before the load. See > the ARM and Alpha targets for examples. This is what gen_load_store_alignment does. > Case (2) and (3) are both handled by using ALIGNED_ONLY. All you > need to do to handle (3) is *not* throw an exception from the > do_unaligned_access function. See how the code is structured > inside softmmu_template.h. > > As for sync insns... You may need to handle it all out-of-line > and check for alignment there. This is also done by gen_load_store_alignment. Does it really worth copying part of this logic to do_unaligned_access just to use ALIGNED_ONLY framework? Thanks. -- Max
On 05/22/2011 01:12 PM, Max Filippov wrote: > This is also done by gen_load_store_alignment. > Does it really worth copying part of this logic to do_unaligned_access just to use ALIGNED_ONLY framework? Yes, because it is done out-of-line, as a part of the TLB load slow path. r~
> > This is also done by gen_load_store_alignment. > > Does it really worth copying part of this logic to do_unaligned_access just to use ALIGNED_ONLY framework? > > Yes, because it is done out-of-line, as a part of the TLB load slow path. I probably just don't get what you call 'out-of-line'. In fact do_unaligned_access will be called for every unaligned access, and alignment condition will be checked for every access. It just happens in other place. Does it have more chances to be optimized better than TCG code, or is it less TCG code itself that makes difference? Thanks. -- Max
On 05/23/2011 04:20 PM, Max Filippov wrote: > I probably just don't get what you call 'out-of-line'. In fact > do_unaligned_access will be called for every unaligned access, and > alignment condition will be checked for every access. It just happens > in other place. Does it have more chances to be optimized better than > TCG code, or is it less TCG code itself that makes difference? Out of line meaning not inside TCG code at all. The fast-path through a tcg_qemu_ld operation handles aligned memory accesses that hit the TLB. Unaligned accesses or TLB misses go to the slow path, in __ld[bwlq]_mmu. It is in these functions that we test for ALIGNED_ONLY, which enables the do_unaligned_access hook. r~
diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index 592072a..6e66f3f 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -177,6 +177,16 @@ static void gen_exception_cause(DisasContext *dc, uint32_t cause) tcg_temp_free(_cause); } +static void gen_exception_cause_vaddr(DisasContext *dc, uint32_t cause, + TCGv_i32 vaddr) +{ + TCGv_i32 _pc = tcg_const_i32(dc->pc); + TCGv_i32 _cause = tcg_const_i32(cause); + gen_helper_exception_cause_vaddr(_pc, _cause, vaddr); + tcg_temp_free(_pc); + tcg_temp_free(_cause); +} + static void gen_check_privilege(DisasContext *dc) { if (dc->mem_idx) { @@ -349,6 +359,20 @@ static void gen_wsr(DisasContext *dc, uint32_t sr, TCGv_i32 s) } } +static void gen_load_store_alignment(DisasContext *dc, int shift, TCGv_i32 addr) +{ + TCGv_i32 tmp = tcg_temp_local_new_i32(); + tcg_gen_mov_i32(tmp, addr); + tcg_gen_andi_i32(addr, addr, ~0 << shift); + if (option_enabled(dc, XTENSA_OPTION_UNALIGNED_EXCEPTION)) { + int label = gen_new_label(); + tcg_gen_brcond_i32(TCG_COND_EQ, addr, tmp, label); + gen_exception_cause_vaddr(dc, LOAD_STORE_ALIGNMENT_CAUSE, tmp); + gen_set_label(label); + } + tcg_temp_free(tmp); +} + static void disas_xtensa_insn(DisasContext *dc) { #define HAS_OPTION(opt) do { \ @@ -1272,8 +1296,11 @@ static void disas_xtensa_insn(DisasContext *dc) case 2: /*LSAI*/ #define gen_load_store(type, shift) do { \ - TCGv_i32 addr = tcg_temp_new_i32(); \ + TCGv_i32 addr = tcg_temp_local_new_i32(); \ tcg_gen_addi_i32(addr, cpu_R[RRI8_S], RRI8_IMM8 << shift); \ + if (shift) { \ + gen_load_store_alignment(dc, shift, addr); \ + } \ tcg_gen_qemu_##type(cpu_R[RRI8_T], addr, 0); \ tcg_temp_free(addr); \ } while (0) @@ -1432,6 +1459,7 @@ static void disas_xtensa_insn(DisasContext *dc) tcg_gen_mov_i32(tmp, cpu_R[RRI8_T]); tcg_gen_addi_i32(addr, cpu_R[RRI8_S], RRI8_IMM8 << 2); + gen_load_store_alignment(dc, 2, addr); tcg_gen_qemu_ld32u(cpu_R[RRI8_T], addr, 0); tcg_gen_brcond_i32(TCG_COND_NE, tmp, cpu_SR[SCOMPARE1], label); @@ -1657,8 +1685,9 @@ static void disas_xtensa_insn(DisasContext *dc) break; #define gen_narrow_load_store(type) do { \ - TCGv_i32 addr = tcg_temp_new_i32(); \ + TCGv_i32 addr = tcg_temp_local_new_i32(); \ tcg_gen_addi_i32(addr, cpu_R[RRRN_S], RRRN_R << 2); \ + gen_load_store_alignment(dc, 2, addr); \ tcg_gen_qemu_##type(cpu_R[RRRN_T], addr, 0); \ tcg_temp_free(addr); \ } while (0)
See ISA, 4.4.4 for details. Correct (aligned as per ISA) address for unaligned access is generated in case this option is not enabled. Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> --- target-xtensa/translate.c | 33 +++++++++++++++++++++++++++++++-- 1 files changed, 31 insertions(+), 2 deletions(-)