On Thu, Dec 17, 2015 at 5:52 PM, Alex Bennée <alex.ben...@linaro.org> wrote: > > Alvise Rigo <a.r...@virtualopensystems.com> writes: > >> Attempting to simplify the helper_*_st_name, wrap the code relative to a >> RAM access into an inline function. > > This commit breaks a default x86_64-softmmu build:
I see. Would these three commits make more sense if squashed together? Or better to leave them distinct and fix the compilation issue? BTW, I will now start using that script. Thank you, alvise > > CC x86_64-softmmu/../hw/audio/pcspk.o > In file included from /home/alex/lsrc/qemu/qemu.git/cputlb.c:527:0: > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h: In function > ‘helper_ret_stb_mmu’: > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h:503:13: error: ‘haddr’ > undeclared (first use in this function) > haddr = addr + env->tlb_table[mmu_idx][index].addend; > ^ > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h:503:13: note: each > undeclared identifier is reported only once for each function it appears in > In file included from /home/alex/lsrc/qemu/qemu.git/cputlb.c:530:0: > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h: In function > ‘helper_le_stw_mmu’: > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h:503:13: error: ‘haddr’ > undeclared (first use in this function) > haddr = addr + env->tlb_table[mmu_idx][index].addend; > ^ > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h: In function > ‘helper_be_stw_mmu’: > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h:651:13: error: ‘haddr’ > undeclared (first use in this function) > haddr = addr + env->tlb_table[mmu_idx][index].addend; > ^ > In file included from /home/alex/lsrc/qemu/qemu.git/cputlb.c:533:0: > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h: In function > ‘helper_le_stl_mmu’: > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h:503:13: error: ‘haddr’ > undeclared (first use in this function) > haddr = addr + env->tlb_table[mmu_idx][index].addend; > ^ > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h: In function > ‘helper_be_stl_mmu’: > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h:651:13: error: ‘haddr’ > undeclared (first use in this function) > haddr = addr + env->tlb_table[mmu_idx][index].addend; > ^ > In file included from /home/alex/lsrc/qemu/qemu.git/cputlb.c:536:0: > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h: In function > ‘helper_le_stq_mmu’: > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h:503:13: error: ‘haddr’ > undeclared (first use in this function) > haddr = addr + env->tlb_table[mmu_idx][index].addend; > ^ > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h: In function > ‘helper_be_stq_mmu’: > /home/alex/lsrc/qemu/qemu.git/softmmu_template.h:651:13: error: ‘haddr’ > undeclared (first use in this function) > haddr = addr + env->tlb_table[mmu_idx][index].addend; > ^ > make[1]: *** [cputlb.o] Error 1 > make[1]: *** Waiting for unfinished jobs.... > CC x86_64-softmmu/../hw/block/fdc.o > make: *** [subdir-x86_64-softmmu] Error 2 > > ERROR: commit 3a371deaf11ce944127a00eadbc7e811b6798de1 failed to build! > commit 3a371deaf11ce944127a00eadbc7e811b6798de1 > Author: Alvise Rigo <a.r...@virtualopensystems.com> > Date: Thu Dec 10 17:26:54 2015 +0100 > > softmmu: Simplify helper_*_st_name, wrap RAM code > > Found while checking with Jeff's compile-check script: > > https://github.com/codyprime/git-scripts > > git compile-check -r c3626ca7df027dabf0568284360a23faf18f0884..HEAD > >> >> Suggested-by: Jani Kokkonen <jani.kokko...@huawei.com> >> Suggested-by: Claudio Fontana <claudio.font...@huawei.com> >> Signed-off-by: Alvise Rigo <a.r...@virtualopensystems.com> >> --- >> softmmu_template.h | 110 >> +++++++++++++++++++++++++++++++++-------------------- >> 1 file changed, 68 insertions(+), 42 deletions(-) >> >> diff --git a/softmmu_template.h b/softmmu_template.h >> index 2ebf527..262c95f 100644 >> --- a/softmmu_template.h >> +++ b/softmmu_template.h >> @@ -416,13 +416,46 @@ static inline void glue(helper_le_st_name, >> _do_mmio_access)(CPUArchState *env, >> glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr); >> } >> >> +static inline void glue(helper_le_st_name, _do_ram_access)(CPUArchState >> *env, >> + DATA_TYPE val, >> + target_ulong >> addr, >> + TCGMemOpIdx oi, >> + unsigned mmu_idx, >> + int index, >> + uintptr_t >> retaddr) >> +{ >> + uintptr_t haddr; >> + >> + /* Handle slow unaligned access (it spans two pages or IO). */ >> + if (DATA_SIZE > 1 >> + && unlikely((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1 >> + >= TARGET_PAGE_SIZE)) { >> + glue(helper_le_st_name, _do_unl_access)(env, val, addr, oi, mmu_idx, >> + retaddr); >> + return; >> + } >> + >> + /* Handle aligned access or unaligned access in the same page. */ >> + if ((addr & (DATA_SIZE - 1)) != 0 >> + && (get_memop(oi) & MO_AMASK) == MO_ALIGN) { >> + cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE, >> + mmu_idx, retaddr); >> + } >> + >> + haddr = addr + env->tlb_table[mmu_idx][index].addend; >> +#if DATA_SIZE == 1 >> + glue(glue(st, SUFFIX), _p)((uint8_t *)haddr, val); >> +#else >> + glue(glue(st, SUFFIX), _le_p)((uint8_t *)haddr, val); >> +#endif >> +} >> + >> void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, >> TCGMemOpIdx oi, uintptr_t retaddr) >> { >> unsigned mmu_idx = get_mmuidx(oi); >> int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); >> target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; >> - uintptr_t haddr; >> >> /* Adjust the given return address. */ >> retaddr -= GETPC_ADJ; >> @@ -484,28 +517,8 @@ void helper_le_st_name(CPUArchState *env, target_ulong >> addr, DATA_TYPE val, >> } >> } >> >> - /* Handle slow unaligned access (it spans two pages or IO). */ >> - if (DATA_SIZE > 1 >> - && unlikely((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1 >> - >= TARGET_PAGE_SIZE)) { >> - glue(helper_le_st_name, _do_unl_access)(env, val, addr, oi, mmu_idx, >> - retaddr); >> - return; >> - } >> - >> - /* Handle aligned access or unaligned access in the same page. */ >> - if ((addr & (DATA_SIZE - 1)) != 0 >> - && (get_memop(oi) & MO_AMASK) == MO_ALIGN) { >> - cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE, >> - mmu_idx, retaddr); >> - } >> - >> - haddr = addr + env->tlb_table[mmu_idx][index].addend; >> -#if DATA_SIZE == 1 >> - glue(glue(st, SUFFIX), _p)((uint8_t *)haddr, val); >> -#else >> - glue(glue(st, SUFFIX), _le_p)((uint8_t *)haddr, val); >> -#endif >> + glue(helper_le_st_name, _do_ram_access)(env, val, addr, oi, mmu_idx, >> index, >> + retaddr); >> } >> >> #if DATA_SIZE > 1 >> @@ -555,13 +568,42 @@ static inline void glue(helper_be_st_name, >> _do_mmio_access)(CPUArchState *env, >> glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr); >> } >> >> +static inline void glue(helper_be_st_name, _do_ram_access)(CPUArchState >> *env, >> + DATA_TYPE val, >> + target_ulong >> addr, >> + TCGMemOpIdx oi, >> + unsigned mmu_idx, >> + int index, >> + uintptr_t >> retaddr) >> +{ >> + uintptr_t haddr; >> + >> + /* Handle slow unaligned access (it spans two pages or IO). */ >> + if (DATA_SIZE > 1 >> + && unlikely((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1 >> + >= TARGET_PAGE_SIZE)) { >> + glue(helper_be_st_name, _do_unl_access)(env, val, addr, oi, mmu_idx, >> + retaddr); >> + return; >> + } >> + >> + /* Handle aligned access or unaligned access in the same page. */ >> + if ((addr & (DATA_SIZE - 1)) != 0 >> + && (get_memop(oi) & MO_AMASK) == MO_ALIGN) { >> + cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE, >> + mmu_idx, retaddr); >> + } >> + >> + haddr = addr + env->tlb_table[mmu_idx][index].addend; >> + glue(glue(st, SUFFIX), _be_p)((uint8_t *)haddr, val); >> +} >> + >> void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, >> TCGMemOpIdx oi, uintptr_t retaddr) >> { >> unsigned mmu_idx = get_mmuidx(oi); >> int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); >> target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; >> - uintptr_t haddr; >> >> /* Adjust the given return address. */ >> retaddr -= GETPC_ADJ; >> @@ -623,24 +665,8 @@ void helper_be_st_name(CPUArchState *env, target_ulong >> addr, DATA_TYPE val, >> } >> } >> >> - /* Handle slow unaligned access (it spans two pages or IO). */ >> - if (DATA_SIZE > 1 >> - && unlikely((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1 >> - >= TARGET_PAGE_SIZE)) { >> - glue(helper_be_st_name, _do_unl_access)(env, val, addr, oi, mmu_idx, >> - retaddr); >> - return; >> - } >> - >> - /* Handle aligned access or unaligned access in the same page. */ >> - if ((addr & (DATA_SIZE - 1)) != 0 >> - && (get_memop(oi) & MO_AMASK) == MO_ALIGN) { >> - cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE, >> - mmu_idx, retaddr); >> - } >> - >> - haddr = addr + env->tlb_table[mmu_idx][index].addend; >> - glue(glue(st, SUFFIX), _be_p)((uint8_t *)haddr, val); >> + glue(helper_be_st_name, _do_ram_access)(env, val, addr, oi, mmu_idx, >> index, >> + retaddr); >> } >> #endif /* DATA_SIZE > 1 */ > > > -- > Alex Bennée