On 1/22/21 10:59 PM, Taylor Simpson wrote: >> -----Original Message----- >> From: Philippe Mathieu-Daudé <philippe.mathieu.da...@gmail.com> On >> Behalf Of Philippe Mathieu-Daudé >> Sent: Friday, January 22, 2021 12:09 PM >> To: Taylor Simpson <tsimp...@quicinc.com>; qemu-devel@nongnu.org >> Cc: richard.hender...@linaro.org; alex.ben...@linaro.org; >> laur...@vivier.eu; a...@rev.ng; Brian Cain <bc...@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 <tsimp...@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.