On 10/8/20 10:00 AM, Taylor Simpson wrote: > > >> -----Original Message----- >> From: Richard Henderson <richard.hender...@linaro.org> >> Sent: Friday, August 28, 2020 7:17 PM >> To: Taylor Simpson <tsimp...@quicinc.com>; qemu-devel@nongnu.org >> Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi; >> aleksandar.m.m...@gmail.com; a...@rev.ng >> Subject: Re: [RFC PATCH v3 26/34] Hexagon (target/hexagon) macros >> referenced in instruction semantics >> >> >> Ah, so I see what you're trying to do with the merge thing, which had the >> host-endian problems. >> >> I think the merge stuff is a mistake. I think you can get the semantics that >> you want with >> >> probe_read(ld_addr, ld_len) >> qemu_st(st_value, st_addr) >> qemu_ld(ld_value, ld_addr) >> >> In this way, all exceptions are recognized before the store is complete, the >> normal memory operations handle any possible overlap. > > Following up on this... > > 1) We don't need to do the probe_read because the load has already happened > at this point.
What do you mean "has already happened"? How can it have done without doing the merging by hand. Which this operation ordering is intended to make unnecessary. I think you're missing the point. > 2) I'm not familiar with qemu_st/qemu_ld. Are these shorthand for > tcg_gen_qemu_st*/tcg_gen_qemu_ld*? Yes. > We can't actually do the store at this point because it would alter the > memory before we are sure the packet will commit (i.e., there might be still > be an exception raised by another instruction in the packet). What other instruction? Give me a concrete example so that I can give you a concrete answer. I think you may need to do more preprocessing of the entire packet so that you can answer this sort of question (is there any other possible exception) when generating code. > 3) How important is it to support big endian hosts? Would it be OK to put > something like this to declare it isn't supported for Hexagon? > #if defined(HOST_WORDS_BIGENDIAN) > #error Hexagon guest not supported on big endian host > #endif That would make ./configure && make fail on such a host. So, no, you can't do that. > > 4) If the above is not OK, are the macros in bswap.h the correct ones to use? > Is this pseudo-code correct? > store_val = le32_to_cpu(store_val); > load_val = le32_to_cpu(load_val); > <merge bytes> > /* store_val is dead so no need to convert back */ > load_val = cpu_to_le32(load_val) There's some misuse of cpu_to_le32 vs le32_to_cpu there (I've never liked those names, but it helps to think about what form the data is already in). r~
