Re: [PATCH] target/arm: Move minor arithmetic helpers out of helper.c

2025-01-10 Thread Peter Maydell
On Tue, 17 Dec 2024 at 18:22, Peter Maydell  wrote:
>
> On Tue, 17 Dec 2024 at 18:17, Richard Henderson
>  wrote:
> >
> > On 12/17/24 11:44, Peter Maydell wrote:
> > > helper.c includes some small TCG helper functions used for mostly
> > > arithmetic instructions.  These are TCG only and there's no need for
> > > them to be in the large and unwieldy helper.c.  Move them out to
> > > their own source file in the tcg/ subdirectory, together with the
> > > op_addsub.h multiply-included template header that they use.
> > >
> > > (Ironically, this means that helper.c no longer contains
> > > any TCG helper function definitions at all.)
> > >
> > > Signed-off-by: Peter Maydell 
> > > ---
> > > One last tiny cleanup for the end of the year...this is a pure
> > > code movement, with no changes.
> >
> > Because of pure code movement,
> > Reviewed-by: Richard Henderson 
> >
> > Ideally op_addsub.h would become tcg/op_addsub.h.inc too.
>
> Yeah, I did think about that but decided to be lazy :-)

I changed my mind on this one over the Christmas break:
since we're moving op_addsub.h it's better to move it directly
to the filename that we want. It should be op_addsub.c.inc,
though, not .h.inc.

-- PMM



Re: [PATCH] target/arm: Move minor arithmetic helpers out of helper.c

2024-12-17 Thread Peter Maydell
On Tue, 17 Dec 2024 at 18:17, Richard Henderson
 wrote:
>
> On 12/17/24 11:44, Peter Maydell wrote:
> > helper.c includes some small TCG helper functions used for mostly
> > arithmetic instructions.  These are TCG only and there's no need for
> > them to be in the large and unwieldy helper.c.  Move them out to
> > their own source file in the tcg/ subdirectory, together with the
> > op_addsub.h multiply-included template header that they use.
> >
> > (Ironically, this means that helper.c no longer contains
> > any TCG helper function definitions at all.)
> >
> > Signed-off-by: Peter Maydell 
> > ---
> > One last tiny cleanup for the end of the year...this is a pure
> > code movement, with no changes.
>
> Because of pure code movement,
> Reviewed-by: Richard Henderson 
>
> Ideally op_addsub.h would become tcg/op_addsub.h.inc too.

Yeah, I did think about that but decided to be lazy :-)

> > +/*
> > + * Note that signed overflow is undefined in C.  The following routines are
> > + * careful to use unsigned types where modulo arithmetic is required.
> > + * Failure to do so _will_ break on newer gcc.
> > + */
>
> The comment is out-of-date vs -fwrapv, which we use.
>
> There are a bunch of other saturation related macros/functions hanging about. 
>  We should
> probably unify them.

Mmmm; this is all rather elderly code from the A32 decoder,
though. I'm not sure it hugely merits much effort. My
aim here was really just to do another little bit
towards slimming down helper.c.

-- PMM



Re: [PATCH] target/arm: Move minor arithmetic helpers out of helper.c

2024-12-17 Thread Richard Henderson

On 12/17/24 11:44, Peter Maydell wrote:

helper.c includes some small TCG helper functions used for mostly
arithmetic instructions.  These are TCG only and there's no need for
them to be in the large and unwieldy helper.c.  Move them out to
their own source file in the tcg/ subdirectory, together with the
op_addsub.h multiply-included template header that they use.

(Ironically, this means that helper.c no longer contains
any TCG helper function definitions at all.)

Signed-off-by: Peter Maydell 
---
One last tiny cleanup for the end of the year...this is a pure
code movement, with no changes.


Because of pure code movement,
Reviewed-by: Richard Henderson 

Ideally op_addsub.h would become tcg/op_addsub.h.inc too.


+/*
+ * Note that signed overflow is undefined in C.  The following routines are
+ * careful to use unsigned types where modulo arithmetic is required.
+ * Failure to do so _will_ break on newer gcc.
+ */


The comment is out-of-date vs -fwrapv, which we use.

There are a bunch of other saturation related macros/functions hanging about.  We should 
probably unify them.



r~