| Message ID | 20230606141252.95032-3-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | misc: Enforce .[ch].inc extension for re-included .c/.h files | expand |
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~
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~
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
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~
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
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%)