Re: [RFC PATCH v3 26/34] Hexagon (target/hexagon) macros referenced in instruction semantics

[Richard Henderson]

On 10/8/20 10:00 AM, Taylor Simpson wrote:
> 
> 
>> -----Original Message-----
>> From: Richard Henderson <richard.henderson@linaro.org>
>> Sent: Friday, August 28, 2020 7:17 PM
>> 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.


> 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~