diff mbox series

[v3,24/48] tcg: Split helper-proto.h

Message ID 20230531040330.8950-25-richard.henderson@linaro.org
State Superseded
Headers show
Series tcg: Build once for system, once for user | expand

Commit Message

Richard Henderson May 31, 2023, 4:03 a.m. UTC 
Create helper-proto-common.h without the target specific portion.
Use that in tcg-op-common.h.  Include helper-proto.h in target/arm
and target/hexagon before helper-info.c.inc; all other targets are
already correct in this regard.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/helper-proto-common.h | 17 +++++++
 include/exec/helper-proto.h        | 72 ++++--------------------------
 include/tcg/tcg-op-common.h        |  2 +-
 include/exec/helper-proto.h.inc    | 67 +++++++++++++++++++++++++++
 accel/tcg/cputlb.c                 |  3 +-
 accel/tcg/plugin-gen.c             |  2 +-
 accel/tcg/tcg-runtime-gvec.c       |  2 +-
 accel/tcg/tcg-runtime.c            |  2 +-
 target/arm/tcg/translate.c         |  1 +
 target/hexagon/translate.c         |  1 +
 10 files changed, 99 insertions(+), 70 deletions(-)
 create mode 100644 include/exec/helper-proto-common.h
 create mode 100644 include/exec/helper-proto.h.inc

Comments

Philippe Mathieu-Daudé June 2, 2023, 9:14 p.m. UTC | #1 
On 31/5/23 06:03, Richard Henderson wrote:
> Create helper-proto-common.h without the target specific portion.
> Use that in tcg-op-common.h.  Include helper-proto.h in target/arm
> and target/hexagon before helper-info.c.inc; all other targets are
> already correct in this regard.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/helper-proto-common.h | 17 +++++++
>   include/exec/helper-proto.h        | 72 ++++--------------------------
>   include/tcg/tcg-op-common.h        |  2 +-
>   include/exec/helper-proto.h.inc    | 67 +++++++++++++++++++++++++++
>   accel/tcg/cputlb.c                 |  3 +-
>   accel/tcg/plugin-gen.c             |  2 +-
>   accel/tcg/tcg-runtime-gvec.c       |  2 +-
>   accel/tcg/tcg-runtime.c            |  2 +-
>   target/arm/tcg/translate.c         |  1 +
>   target/hexagon/translate.c         |  1 +
>   10 files changed, 99 insertions(+), 70 deletions(-)
>   create mode 100644 include/exec/helper-proto-common.h
>   create mode 100644 include/exec/helper-proto.h.inc


> diff --git a/include/exec/helper-proto.h.inc b/include/exec/helper-proto.h.inc
> new file mode 100644
> index 0000000000..f6f0cfcacd
> --- /dev/null
> +++ b/include/exec/helper-proto.h.inc
> @@ -0,0 +1,67 @@
> +/*
> + * Helper file for declaring TCG helper functions.
> + * This one expands prototypes for the helper functions.
> + * Define HELPER_H for the header file to be expanded.
> + */
> +
> +#include "exec/helper-head.h"
> +
> +/*
> + * Work around an issue with --enable-lto, in which GCC's ipa-split pass
> + * decides to split out the noreturn code paths that raise an exception,
> + * taking the __builtin_return_address() along into the new function,
> + * where it no longer computes a value that returns to TCG generated code.
> + * Despite the name, the noinline attribute affects splitter, so this
> + * prevents the optimization in question.  Given that helpers should not
> + * otherwise be called directly, this should have any other visible effect.
> + *
> + * See https://gitlab.com/qemu-project/qemu/-/issues/1454
> + */
> +#define DEF_HELPER_ATTR  __attribute__((noinline))
> +
> +#define DEF_HELPER_FLAGS_0(name, flags, ret) \
> +dh_ctype(ret) HELPER(name) (void) DEF_HELPER_ATTR;
> +
> +#define DEF_HELPER_FLAGS_1(name, flags, ret, t1) \
> +dh_ctype(ret) HELPER(name) (dh_ctype(t1)) DEF_HELPER_ATTR;
> +
> +#define DEF_HELPER_FLAGS_2(name, flags, ret, t1, t2) \
> +dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2)) DEF_HELPER_ATTR;
> +
> +#define DEF_HELPER_FLAGS_3(name, flags, ret, t1, t2, t3) \
> +dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), \
> +                            dh_ctype(t3)) DEF_HELPER_ATTR;
> +
> +#define DEF_HELPER_FLAGS_4(name, flags, ret, t1, t2, t3, t4) \
> +dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
> +                            dh_ctype(t4)) DEF_HELPER_ATTR;
> +
> +#define DEF_HELPER_FLAGS_5(name, flags, ret, t1, t2, t3, t4, t5) \
> +dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
> +                            dh_ctype(t4), dh_ctype(t5)) DEF_HELPER_ATTR;
> +
> +#define DEF_HELPER_FLAGS_6(name, flags, ret, t1, t2, t3, t4, t5, t6) \
> +dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
> +                            dh_ctype(t4), dh_ctype(t5), \
> +                            dh_ctype(t6)) DEF_HELPER_ATTR;
> +
> +#define DEF_HELPER_FLAGS_7(name, flags, ret, t1, t2, t3, t4, t5, t6, t7) \
> +dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
> +                            dh_ctype(t4), dh_ctype(t5), dh_ctype(t6), \
> +                            dh_ctype(t7)) DEF_HELPER_ATTR;
> +
> +#define IN_HELPER_PROTO
> +
> +#include HELPER_H
> +
> +#undef IN_HELPER_PROTO
> +
> +#undef DEF_HELPER_FLAGS_0
> +#undef DEF_HELPER_FLAGS_1
> +#undef DEF_HELPER_FLAGS_2
> +#undef DEF_HELPER_FLAGS_3
> +#undef DEF_HELPER_FLAGS_4
> +#undef DEF_HELPER_FLAGS_5
> +#undef DEF_HELPER_FLAGS_6
> +#undef DEF_HELPER_FLAGS_7
> +#undef DEF_HELPER_ATTR

Should we guard this header for multiple inclusions?

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Richard Henderson June 3, 2023, 4:10 a.m. UTC | #2 
On 6/2/23 14:14, Philippe Mathieu-Daudé wrote:
> On 31/5/23 06:03, Richard Henderson wrote:
>> Create helper-proto-common.h without the target specific portion.
>> Use that in tcg-op-common.h.  Include helper-proto.h in target/arm
>> and target/hexagon before helper-info.c.inc; all other targets are
>> already correct in this regard.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   include/exec/helper-proto-common.h | 17 +++++++
>>   include/exec/helper-proto.h        | 72 ++++--------------------------
>>   include/tcg/tcg-op-common.h        |  2 +-
>>   include/exec/helper-proto.h.inc    | 67 +++++++++++++++++++++++++++
>>   accel/tcg/cputlb.c                 |  3 +-
>>   accel/tcg/plugin-gen.c             |  2 +-
>>   accel/tcg/tcg-runtime-gvec.c       |  2 +-
>>   accel/tcg/tcg-runtime.c            |  2 +-
>>   target/arm/tcg/translate.c         |  1 +
>>   target/hexagon/translate.c         |  1 +
>>   10 files changed, 99 insertions(+), 70 deletions(-)
>>   create mode 100644 include/exec/helper-proto-common.h
>>   create mode 100644 include/exec/helper-proto.h.inc
> 
> 
>> diff --git a/include/exec/helper-proto.h.inc b/include/exec/helper-proto.h.inc
>> new file mode 100644
>> index 0000000000..f6f0cfcacd
>> --- /dev/null
>> +++ b/include/exec/helper-proto.h.inc
...

> Should we guard this header for multiple inclusions?

No, *.h.inc again.


r~
diff mbox series

Patch

diff --git a/include/exec/helper-proto-common.h b/include/exec/helper-proto-common.h
new file mode 100644
index 0000000000..666778473e
--- /dev/null
+++ b/include/exec/helper-proto-common.h
@@ -0,0 +1,17 @@ 
+/*
+ * Helper file for declaring TCG helper functions.
+ * This one expands prototypes for the helper functions.
+ */
+
+#ifndef HELPER_PROTO_COMMON_H
+#define HELPER_PROTO_COMMON_H
+
+#define HELPER_H "accel/tcg/tcg-runtime.h"
+#include "exec/helper-proto.h.inc"
+#undef  HELPER_H
+
+#define HELPER_H "accel/tcg/plugin-helpers.h"
+#include "exec/helper-proto.h.inc"
+#undef  HELPER_H
+
+#endif /* HELPER_PROTO_COMMON_H */
diff --git a/include/exec/helper-proto.h b/include/exec/helper-proto.h
index 7a3f04b58c..aac684dbbf 100644
--- a/include/exec/helper-proto.h
+++ b/include/exec/helper-proto.h
@@ -1,71 +1,15 @@ 
-/* Helper file for declaring TCG helper functions.
-   This one expands prototypes for the helper functions.  */
+/*
+ * Helper file for declaring TCG helper functions.
+ * This one expands prototypes for the helper functions.
+ */
 
 #ifndef HELPER_PROTO_H
 #define HELPER_PROTO_H
 
-#include "exec/helper-head.h"
+#include "exec/helper-proto-common.h"
 
-/*
- * Work around an issue with --enable-lto, in which GCC's ipa-split pass
- * decides to split out the noreturn code paths that raise an exception,
- * taking the __builtin_return_address() along into the new function,
- * where it no longer computes a value that returns to TCG generated code.
- * Despite the name, the noinline attribute affects splitter, so this
- * prevents the optimization in question.  Given that helpers should not
- * otherwise be called directly, this should have any other visible effect.
- *
- * See https://gitlab.com/qemu-project/qemu/-/issues/1454
- */
-#define DEF_HELPER_ATTR  __attribute__((noinline))
-
-#define DEF_HELPER_FLAGS_0(name, flags, ret) \
-dh_ctype(ret) HELPER(name) (void) DEF_HELPER_ATTR;
-
-#define DEF_HELPER_FLAGS_1(name, flags, ret, t1) \
-dh_ctype(ret) HELPER(name) (dh_ctype(t1)) DEF_HELPER_ATTR;
-
-#define DEF_HELPER_FLAGS_2(name, flags, ret, t1, t2) \
-dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2)) DEF_HELPER_ATTR;
-
-#define DEF_HELPER_FLAGS_3(name, flags, ret, t1, t2, t3) \
-dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), \
-                            dh_ctype(t3)) DEF_HELPER_ATTR;
-
-#define DEF_HELPER_FLAGS_4(name, flags, ret, t1, t2, t3, t4) \
-dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
-                            dh_ctype(t4)) DEF_HELPER_ATTR;
-
-#define DEF_HELPER_FLAGS_5(name, flags, ret, t1, t2, t3, t4, t5) \
-dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
-                            dh_ctype(t4), dh_ctype(t5)) DEF_HELPER_ATTR;
-
-#define DEF_HELPER_FLAGS_6(name, flags, ret, t1, t2, t3, t4, t5, t6) \
-dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
-                            dh_ctype(t4), dh_ctype(t5), \
-                            dh_ctype(t6)) DEF_HELPER_ATTR;
-
-#define DEF_HELPER_FLAGS_7(name, flags, ret, t1, t2, t3, t4, t5, t6, t7) \
-dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
-                            dh_ctype(t4), dh_ctype(t5), dh_ctype(t6), \
-                            dh_ctype(t7)) DEF_HELPER_ATTR;
-
-#define IN_HELPER_PROTO
-
-#include "helper.h"
-#include "accel/tcg/tcg-runtime.h"
-#include "accel/tcg/plugin-helpers.h"
-
-#undef IN_HELPER_PROTO
-
-#undef DEF_HELPER_FLAGS_0
-#undef DEF_HELPER_FLAGS_1
-#undef DEF_HELPER_FLAGS_2
-#undef DEF_HELPER_FLAGS_3
-#undef DEF_HELPER_FLAGS_4
-#undef DEF_HELPER_FLAGS_5
-#undef DEF_HELPER_FLAGS_6
-#undef DEF_HELPER_FLAGS_7
-#undef DEF_HELPER_ATTR
+#define HELPER_H "helper.h"
+#include "exec/helper-proto.h.inc"
+#undef  HELPER_H
 
 #endif /* HELPER_PROTO_H */
diff --git a/include/tcg/tcg-op-common.h b/include/tcg/tcg-op-common.h
index f6f05469c5..be382bbf77 100644
--- a/include/tcg/tcg-op-common.h
+++ b/include/tcg/tcg-op-common.h
@@ -9,7 +9,7 @@ 
 #define TCG_TCG_OP_COMMON_H
 
 #include "tcg/tcg.h"
-#include "exec/helper-proto.h"
+#include "exec/helper-proto-common.h"
 #include "exec/helper-gen-common.h"
 
 /* Basic output routines.  Not for general consumption.  */
diff --git a/include/exec/helper-proto.h.inc b/include/exec/helper-proto.h.inc
new file mode 100644
index 0000000000..f6f0cfcacd
--- /dev/null
+++ b/include/exec/helper-proto.h.inc
@@ -0,0 +1,67 @@ 
+/*
+ * Helper file for declaring TCG helper functions.
+ * This one expands prototypes for the helper functions.
+ * Define HELPER_H for the header file to be expanded.
+ */
+
+#include "exec/helper-head.h"
+
+/*
+ * Work around an issue with --enable-lto, in which GCC's ipa-split pass
+ * decides to split out the noreturn code paths that raise an exception,
+ * taking the __builtin_return_address() along into the new function,
+ * where it no longer computes a value that returns to TCG generated code.
+ * Despite the name, the noinline attribute affects splitter, so this
+ * prevents the optimization in question.  Given that helpers should not
+ * otherwise be called directly, this should have any other visible effect.
+ *
+ * See https://gitlab.com/qemu-project/qemu/-/issues/1454
+ */
+#define DEF_HELPER_ATTR  __attribute__((noinline))
+
+#define DEF_HELPER_FLAGS_0(name, flags, ret) \
+dh_ctype(ret) HELPER(name) (void) DEF_HELPER_ATTR;
+
+#define DEF_HELPER_FLAGS_1(name, flags, ret, t1) \
+dh_ctype(ret) HELPER(name) (dh_ctype(t1)) DEF_HELPER_ATTR;
+
+#define DEF_HELPER_FLAGS_2(name, flags, ret, t1, t2) \
+dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2)) DEF_HELPER_ATTR;
+
+#define DEF_HELPER_FLAGS_3(name, flags, ret, t1, t2, t3) \
+dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), \
+                            dh_ctype(t3)) DEF_HELPER_ATTR;
+
+#define DEF_HELPER_FLAGS_4(name, flags, ret, t1, t2, t3, t4) \
+dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
+                            dh_ctype(t4)) DEF_HELPER_ATTR;
+
+#define DEF_HELPER_FLAGS_5(name, flags, ret, t1, t2, t3, t4, t5) \
+dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
+                            dh_ctype(t4), dh_ctype(t5)) DEF_HELPER_ATTR;
+
+#define DEF_HELPER_FLAGS_6(name, flags, ret, t1, t2, t3, t4, t5, t6) \
+dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
+                            dh_ctype(t4), dh_ctype(t5), \
+                            dh_ctype(t6)) DEF_HELPER_ATTR;
+
+#define DEF_HELPER_FLAGS_7(name, flags, ret, t1, t2, t3, t4, t5, t6, t7) \
+dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
+                            dh_ctype(t4), dh_ctype(t5), dh_ctype(t6), \
+                            dh_ctype(t7)) DEF_HELPER_ATTR;
+
+#define IN_HELPER_PROTO
+
+#include HELPER_H
+
+#undef IN_HELPER_PROTO
+
+#undef DEF_HELPER_FLAGS_0
+#undef DEF_HELPER_FLAGS_1
+#undef DEF_HELPER_FLAGS_2
+#undef DEF_HELPER_FLAGS_3
+#undef DEF_HELPER_FLAGS_4
+#undef DEF_HELPER_FLAGS_5
+#undef DEF_HELPER_FLAGS_6
+#undef DEF_HELPER_FLAGS_7
+#undef DEF_HELPER_ATTR
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 32a4817139..5e2ca47243 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -29,7 +29,7 @@ 
 #include "tcg/tcg.h"
 #include "qemu/error-report.h"
 #include "exec/log.h"
-#include "exec/helper-proto.h"
+#include "exec/helper-proto-common.h"
 #include "qemu/atomic.h"
 #include "qemu/atomic128.h"
 #include "exec/translate-all.h"
@@ -41,7 +41,6 @@ 
 #endif
 #include "tcg/tcg-ldst.h"
 #include "tcg/oversized-guest.h"
-#include "exec/helper-proto.h"
 
 /* DEBUG defines, enable DEBUG_TLB_LOG to log to the CPU_LOG_MMU target */
 /* #define DEBUG_TLB */
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 40b34a0403..3e528f191d 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -49,7 +49,7 @@ 
 #include "exec/exec-all.h"
 #include "exec/plugin-gen.h"
 #include "exec/translator.h"
-#include "exec/helper-proto.h"
+#include "exec/helper-proto-common.h"
 
 #define HELPER_H  "accel/tcg/plugin-helpers.h"
 #include "exec/helper-info.c.inc"
diff --git a/accel/tcg/tcg-runtime-gvec.c b/accel/tcg/tcg-runtime-gvec.c
index 97399493d5..6c99f952ca 100644
--- a/accel/tcg/tcg-runtime-gvec.c
+++ b/accel/tcg/tcg-runtime-gvec.c
@@ -20,7 +20,7 @@ 
 #include "qemu/osdep.h"
 #include "qemu/host-utils.h"
 #include "cpu.h"
-#include "exec/helper-proto.h"
+#include "exec/helper-proto-common.h"
 #include "tcg/tcg-gvec-desc.h"
 
 
diff --git a/accel/tcg/tcg-runtime.c b/accel/tcg/tcg-runtime.c
index 14b59a36e5..9fa539ad3d 100644
--- a/accel/tcg/tcg-runtime.c
+++ b/accel/tcg/tcg-runtime.c
@@ -24,7 +24,7 @@ 
 #include "qemu/osdep.h"
 #include "qemu/host-utils.h"
 #include "cpu.h"
-#include "exec/helper-proto.h"
+#include "exec/helper-proto-common.h"
 #include "exec/cpu_ldst.h"
 #include "exec/exec-all.h"
 #include "disas/disas.h"
diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
index ce50531dff..379f266256 100644
--- a/target/arm/tcg/translate.c
+++ b/target/arm/tcg/translate.c
@@ -35,6 +35,7 @@ 
 #include "translate.h"
 #include "translate-a32.h"
 #include "exec/gen-icount.h"
+#include "exec/helper-proto.h"
 
 #define HELPER_H "helper.h"
 #include "exec/helper-info.c.inc"
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 00e25035ce..770de58647 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -21,6 +21,7 @@ 
 #include "tcg/tcg-op.h"
 #include "tcg/tcg-op-gvec.h"
 #include "exec/helper-gen.h"
+#include "exec/helper-proto.h"
 #include "exec/cpu_ldst.h"
 #include "exec/log.h"
 #include "internal.h"