[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 15/35] Hexagon (target/hexagon/arch.[ch]) utility function
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v7 15/35] Hexagon (target/hexagon/arch.[ch]) utility functions |
Date: |
Mon, 25 Jan 2021 17:29:38 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 |
On 1/22/21 10:59 PM, Taylor Simpson wrote:
>> -----Original Message-----
>> From: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> On
>> Behalf Of Philippe Mathieu-Daudé
>> Sent: Friday, January 22, 2021 12:09 PM
>> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
>> Cc: richard.henderson@linaro.org; alex.bennee@linaro.org;
>> laurent@vivier.eu; ale@rev.ng; Brian Cain <bcain@quicinc.com>
>> Subject: Re: [PATCH v7 15/35] Hexagon (target/hexagon/arch.[ch]) utility
>> functions
>>
>> Hi Taylor,
>>
>> On 1/20/21 4:28 AM, Taylor Simpson wrote:
>>> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
>>> ---
>>> target/hexagon/arch.h | 35 ++++++
>>> target/hexagon/arch.c | 294
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 329 insertions(+)
>>> create mode 100644 target/hexagon/arch.h
>>> create mode 100644 target/hexagon/arch.c
>>>
>>> diff --git a/target/hexagon/arch.h b/target/hexagon/arch.h
>>> new file mode 100644
>>> index 0000000..a8374a3
>>> --- /dev/null
>>> +++ b/target/hexagon/arch.h
>>
>> Maybe rename "arch_utils.[ch]"?
>
> Any particular reason?
I was thinking about not being confused by an architecture specific
file when doing refactors involving multiple architectures. But this
isn't a great improvement neither... No problem.
>
>>
>>> +extern int arch_sf_invsqrt_common(float32 *Rs, float32 *Rd, int *adjust,
>>> + float_status *fp_status);
>>
>> (Again, no need for 'extern').
>
> OK, I will change these.
>
>>> diff --git a/target/hexagon/arch.c b/target/hexagon/arch.c
>>> new file mode 100644
>>> index 0000000..c59cad5
>>> --- /dev/null
>>> +++ b/target/hexagon/arch.c
>> ...
>>
>>> +#define RAISE_FP_EXCEPTION \
>>> + do {} while (0) /* Not modelled in qemu user mode */
>>
>> I don't understand why... Can you explain please?
>
> Our Linux kernel only sets the relevant bits in USR (user status register).
> The exception isn't raised to user mode.
Hmm while you introduce the linux-user implementation of your port,
this file is not restricted to user mode. Thinking about avoiding
head aches to someone wanting to add system mode emulation (or a
BSD port??), maybe your helpers should consider that.
Maybe some cheap #ifdef'ry CONFIG_USER_ONLY with a comment explaining
why there is nothing to do in user mode, and g_assert_not_reached()
else. Not sure, just wondering...
Regards,
Phil.
- [PATCH v7 05/35] Hexagon (disas) disassembler, (continued)
- [PATCH v7 05/35] Hexagon (disas) disassembler, Taylor Simpson, 2021/01/19
- [PATCH v7 35/35] Add Dockerfile for hexagon, Taylor Simpson, 2021/01/19
- [PATCH v7 08/35] Hexagon (target/hexagon) GDB Stub, Taylor Simpson, 2021/01/19
- [PATCH v7 14/35] Hexagon (target/hexagon) instruction printing, Taylor Simpson, 2021/01/19
- [PATCH v7 15/35] Hexagon (target/hexagon/arch.[ch]) utility functions, Taylor Simpson, 2021/01/19
[PATCH v7 19/35] Hexagon (target/hexagon) generator phase 1 - C preprocessor for semantics, Taylor Simpson, 2021/01/19
[PATCH v7 22/35] Hexagon (target/hexagon) generater phase 4 - decode tree, Taylor Simpson, 2021/01/19
[PATCH v7 17/35] Hexagon (target/hexagon/fma_emu.[ch]) utility functions, Taylor Simpson, 2021/01/19
[PATCH v7 18/35] Hexagon (target/hexagon/imported) arch import, Taylor Simpson, 2021/01/19
[PATCH v7 20/35] Hexagon (target/hexagon) generator phase 2 - generate header files, Taylor Simpson, 2021/01/19
[PATCH v7 26/35] Hexagon (target/hexagon) TCG generation, Taylor Simpson, 2021/01/19
[PATCH v7 27/35] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Taylor Simpson, 2021/01/19
[PATCH v7 23/35] Hexagon (target/hexagon) opcode data structures, Taylor Simpson, 2021/01/19
[PATCH v7 29/35] Hexagon (target/hexagon) translation, Taylor Simpson, 2021/01/19