[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [RFC PATCH v3 26/34] Hexagon (target/hexagon) macros referenced in i
From: |
Taylor Simpson |
Subject: |
RE: [RFC PATCH v3 26/34] Hexagon (target/hexagon) macros referenced in instruction semantics |
Date: |
Thu, 8 Oct 2020 18:51:31 +0000 |
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Thursday, October 8, 2020 11:31 AM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: philmd@redhat.com; laurent@vivier.eu; riku.voipio@iki.fi;
> aleksandar.m.mail@gmail.com; ale@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.
Sorry I wasn't clear. We have done the load from memory as it was at the
beginning of the packet. The result of the store is in mem_log_stores in
CPUHexagonState. This code updates the bytes that were loaded with any
overlapping bytes from the store that hasn't been committed yet.
>
>
> > 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.
Remember, there can be 4 instructions in a packet. This code is only dealing
with two of them (a load and a store). Here's an example where a different
instruction in the packet can fault.
67f8: c0 40 21 1f 1f2140c0 { v0.uh = vsat(v0.uw,v1.uw)
67fc: 00 45 02 a1 a1024500 memb(r2+#0) = r5
6800: 02 c0 04 91 9104c002 r2 = memb(r4+#0) }
The vsat instruction requires a vector context. If the thread doesn't have a
vector context, an exception will be raised. The OS can provide a context and
replay the packet.
> 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).
So, what is the right sequence?
Thanks,
Taylor