[PATCH 2/5] target/arm: Rename helper template headers as '.h.inc'

Philippe Mathieu-Daudé posted 5 patches 2 years, 8 months ago
Maintainers: Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Michael Rolnik <mrolnik@gmail.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Brian Cain <bcain@quicinc.com>, Eduardo Habkost <eduardo@habkost.net>, Song Gao <gaosong@loongson.cn>, Xiaojuan Yang <yangxiaojuan@loongson.cn>, Laurent Vivier <laurent@vivier.eu>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Chris Wulff <crwulff@gmail.com>, Marek Vasut <marex@denx.de>, Stafford Horne <shorne@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Max Filippov <jcmvbkbc@gmail.com>
There is a newer version of this series
[PATCH 2/5] target/arm: Rename helper template headers as '.h.inc'
Posted by Philippe Mathieu-Daudé 2 years, 8 months ago
Since commit 139c1837db ("meson: rename included C source files
to .c.inc"), QEMU standard procedure for included C files is to
use *.c.inc.

Besides, since commit 6a0057aa22 ("docs/devel: make a statement
about includes") this is documented as the Coding Style:

  If you do use template header files they should be named with
  the ``.c.inc`` or ``.h.inc`` suffix to make it clear they are
  being included for expansion.

Therefore rename the included templates as '.h.inc'.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/helper.h                               | 8 ++++----
 target/arm/tcg/{helper-a64.h => helper-a64.h.inc} | 0
 target/arm/tcg/{helper-mve.h => helper-mve.h.inc} | 0
 target/arm/tcg/{helper-sme.h => helper-sme.h.inc} | 0
 target/arm/tcg/{helper-sve.h => helper-sve.h.inc} | 0
 5 files changed, 4 insertions(+), 4 deletions(-)
 rename target/arm/tcg/{helper-a64.h => helper-a64.h.inc} (100%)
 rename target/arm/tcg/{helper-mve.h => helper-mve.h.inc} (100%)
 rename target/arm/tcg/{helper-sme.h => helper-sme.h.inc} (100%)
 rename target/arm/tcg/{helper-sve.h => helper-sve.h.inc} (100%)

diff --git a/target/arm/helper.h b/target/arm/helper.h
index 3335c2b10b..4218d98b51 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -1039,9 +1039,9 @@ DEF_HELPER_FLAGS_5(gvec_uclamp_d, TCG_CALL_NO_RWG,
                    void, ptr, ptr, ptr, ptr, i32)
 
 #ifdef TARGET_AARCH64
-#include "tcg/helper-a64.h"
-#include "tcg/helper-sve.h"
-#include "tcg/helper-sme.h"
+#include "tcg/helper-a64.h.inc"
+#include "tcg/helper-sve.h.inc"
+#include "tcg/helper-sme.h.inc"
 #endif
 
-#include "tcg/helper-mve.h"
+#include "tcg/helper-mve.h.inc"
diff --git a/target/arm/tcg/helper-a64.h b/target/arm/tcg/helper-a64.h.inc
similarity index 100%
rename from target/arm/tcg/helper-a64.h
rename to target/arm/tcg/helper-a64.h.inc
diff --git a/target/arm/tcg/helper-mve.h b/target/arm/tcg/helper-mve.h.inc
similarity index 100%
rename from target/arm/tcg/helper-mve.h
rename to target/arm/tcg/helper-mve.h.inc
diff --git a/target/arm/tcg/helper-sme.h b/target/arm/tcg/helper-sme.h.inc
similarity index 100%
rename from target/arm/tcg/helper-sme.h
rename to target/arm/tcg/helper-sme.h.inc
diff --git a/target/arm/tcg/helper-sve.h b/target/arm/tcg/helper-sve.h.inc
similarity index 100%
rename from target/arm/tcg/helper-sve.h
rename to target/arm/tcg/helper-sve.h.inc
-- 
2.38.1


Re: [PATCH 2/5] target/arm: Rename helper template headers as '.h.inc'
Posted by Richard Henderson 2 years, 8 months ago
On 6/6/23 07:12, Philippe Mathieu-Daudé wrote:
> Since commit 139c1837db ("meson: rename included C source files
> to .c.inc"), QEMU standard procedure for included C files is to
> use *.c.inc.
> 
> Besides, since commit 6a0057aa22 ("docs/devel: make a statement
> about includes") this is documented as the Coding Style:
> 
>    If you do use template header files they should be named with
>    the ``.c.inc`` or ``.h.inc`` suffix to make it clear they are
>    being included for expansion.
> 
> Therefore rename the included templates as '.h.inc'.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

FYI, after yesterday's tcg pr, we can do more than this.  These fragments no longer have 
to be all included into one common helper.h. Each translate-foo.c can include only the 
helper-foo.h.inc bits that they need, and the bits need not be visible to the rest of the 
front end.

It was something that I had in mind when splitting include/exec/helper-gen.h, but the 
patch set was already large enough.

The renaming to .h.inc would have been the first step, anyway.


r~

Re: [PATCH 2/5] target/arm: Rename helper template headers as '.h.inc'
Posted by Philippe Mathieu-Daudé 2 years, 8 months ago
On 6/6/23 16:37, Richard Henderson wrote:
> On 6/6/23 07:12, Philippe Mathieu-Daudé wrote:
>> Since commit 139c1837db ("meson: rename included C source files
>> to .c.inc"), QEMU standard procedure for included C files is to
>> use *.c.inc.
>>
>> Besides, since commit 6a0057aa22 ("docs/devel: make a statement
>> about includes") this is documented as the Coding Style:
>>
>>    If you do use template header files they should be named with
>>    the ``.c.inc`` or ``.h.inc`` suffix to make it clear they are
>>    being included for expansion.
>>
>> Therefore rename the included templates as '.h.inc'.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> FYI, after yesterday's tcg pr, we can do more than this.  These 
> fragments no longer have to be all included into one common helper.h. 
> Each translate-foo.c can include only the helper-foo.h.inc bits that 
> they need, and the bits need not be visible to the rest of the front end.

Don't we need foo fully converted to decodetree first? Otherwise
generic translate code can call foo helpers, so needs their prototype
declaration.

For example in translate-a64.c handle_msr_i(SVCR) calls
gen_helper_set_svcr() which is declared in helper-sme.h.

> It was something that I had in mind when splitting 
> include/exec/helper-gen.h, but the patch set was already large enough.
> 
> The renaming to .h.inc would have been the first step, anyway.
> 
> 
> r~


Re: [PATCH 2/5] target/arm: Rename helper template headers as '.h.inc'
Posted by Richard Henderson 2 years, 8 months ago
On 6/6/23 08:49, Philippe Mathieu-Daudé wrote:
> For example in translate-a64.c handle_msr_i(SVCR) calls
> gen_helper_set_svcr() which is declared in helper-sme.h.

The placement in helper-sme.h was not done with consideration as to which compilation 
units might need to use it, since that wasn't a thing at the time.


r~

Re: [PATCH 2/5] target/arm: Rename helper template headers as '.h.inc'
Posted by Peter Maydell 2 years, 8 months ago
On Tue, 6 Jun 2023 at 16:50, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 6/6/23 16:37, Richard Henderson wrote:
> > On 6/6/23 07:12, Philippe Mathieu-Daudé wrote:
> >> Since commit 139c1837db ("meson: rename included C source files
> >> to .c.inc"), QEMU standard procedure for included C files is to
> >> use *.c.inc.
> >>
> >> Besides, since commit 6a0057aa22 ("docs/devel: make a statement
> >> about includes") this is documented as the Coding Style:
> >>
> >>    If you do use template header files they should be named with
> >>    the ``.c.inc`` or ``.h.inc`` suffix to make it clear they are
> >>    being included for expansion.
> >>
> >> Therefore rename the included templates as '.h.inc'.
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> >
> > FYI, after yesterday's tcg pr, we can do more than this.  These
> > fragments no longer have to be all included into one common helper.h.
> > Each translate-foo.c can include only the helper-foo.h.inc bits that
> > they need, and the bits need not be visible to the rest of the front end.
>
> Don't we need foo fully converted to decodetree first? Otherwise
> generic translate code can call foo helpers, so needs their prototype
> declaration.
>
> For example in translate-a64.c handle_msr_i(SVCR) calls
> gen_helper_set_svcr() which is declared in helper-sme.h.

That's unrelated to decodetree -- the decodetree conversion for
that instruction still has code in translate-a64.c which
calls gen_helper_set_svcr(), it's just in a different function.

https://patchew.org/QEMU/20230602155223.2040685-1-peter.maydell@linaro.org/20230602155223.2040685-6-peter.maydell@linaro.org/

thanks
-- PMM