On Sat, Nov 9, 2024 at 8:30 PM Schrodinger ZHU Yifan <i...@zhuyi.fan> wrote:
>
> This patch adds dynamic alloca stubs support to GCCJIT.
>
> DEF_BUILTIN_STUB only define the enum for builtins instead of
> providing the type. Therefore, builtins with stub will lead to
> ICE before this patch. This applies to `alloca_with_align`,
> `stack_save` and `stack_restore`.
>
> This patch add special handling for builtins defined by
> DEF_BUILTIN_STUB. Additionally, it fixes `fold_builtin_with_align`
> by adding an addtional check on the presence of supercontext. For
> blocks created by GCCJIT, such field does not exist while the folder
> implementation assumes the existence on default.
>
> This is my first patch to GCC and GCCJIT. I mainly work on LLVM and other
> github-based projects before. So it is a bit hard for me to get familiar with
> the workflow. I managed to pass the GNU Style check locally but I don't think
> I am
> doing the correct formatting. Also, I tried the changelog script but
> I don't think the patch contains the changelog list. Is it because I did not
> invoke it as a g
> it-hook?
>
> Please refer me to more detailed guideline to correct the format or changelog
> if needed. I am *really really* for the inconvenience.
>
> ---
> gcc/jit/jit-builtins.cc | 66 +++++++-
> gcc/jit/jit-builtins.h | 7 +
> gcc/testsuite/jit.dg/test-aligned-alloca.c | 141 ++++++++++++++++++
> .../jit.dg/test-stack-save-restore.c | 116 ++++++++++++++
> gcc/tree-ssa-ccp.cc | 3 +-
> 5 files changed, 329 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/jit.dg/test-aligned-alloca.c
> create mode 100644 gcc/testsuite/jit.dg/test-stack-save-restore.c
>
> diff --git a/gcc/jit/jit-builtins.cc b/gcc/jit/jit-builtins.cc
> index 0c13c8db586..bf4251701d3 100644
> --- a/gcc/jit/jit-builtins.cc
> +++ b/gcc/jit/jit-builtins.cc
> @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3. If not see
> #include "target.h"
> #include "jit-playback.h"
> #include "stringpool.h"
> +#include "tree-core.h"
>
> #
> include "jit-builtins.h"
>
> @@ -185,7 +186,8 @@ builtins_manager::make_builtin_function (enum
> built_in_function builtin_id)
> {
> const struct builtin_data& bd = builtin_data[builtin_id];
> enum jit_builtin_type type_id = bd.type;
> - recording::type *t = get_type (type_id);
> + recording::type *t = type_id == BT_LAST ? get_type_for_stub (builtin_id)
> + : get_type (type_id);
> if (!t)
> return NULL;
> recording::function_type *func_type = t->as_a_function_type ();
> @@ -333,6 +335,49 @@ builtins_manager::get_type (enum jit_builtin_type
> type_id)
> return m_types[type_id];
> }
>
> +/* Create the recording::type for special builtins whose types are not
> defined
> + in builtin-types.def. */
> +
> +recording::type *
> +builtins_manager::make_type_for_stub (enum built_in_function builtin_id)
> +{
> + switch (builtin_id)
> + {
> + default:
> + return reinterpret_cast<recording::type *>(-1);
> + case BUILT_IN_ALLOCA_WITH_ALIGN: {
> + recording::type * p = m_ctxt->get_
> type (GCC_JIT_TYPE_SIZE_T);
> + recording::type * r = m_ctxt->get_type (GCC_JIT_TYPE_VOID_PTR);
> + recording::type * params[2] = {p, p};
> + return m_ctxt->new_function_type (r,2,params,false);
> + }
> + case BUILT_IN_STACK_SAVE: {
> + recording::type * r = m_ctxt->get_type (GCC_JIT_TYPE_VOID_PTR);
> + return m_ctxt->new_function_type (r,0,nullptr,false);
> + }
> + case BUILT_IN_STACK_RESTORE: {
> + recording::type * p = m_ctxt->get_type (GCC_JIT_TYPE_VOID_PTR);
> + recording::type * r = m_ctxt->get_type (GCC_JIT_TYPE_VOID);
> + recording::type * params[1] = {p};
> + return m_ctxt->new_function_type (r,1,params,false);
> + }
> + }
> +}
> +
> +/* Get the recording::type for a given type of builtin function,
> + by ID, creating it if it doesn't already exist. */
> +
> +recording::type *
> +builtins_manager::get_type_for_stub (enum built_in_function type_id)
> +{
> + if (m_types[type_id] == nullptr)
> + m_types[type_id] = make_type_for_stub (typ
> e_id);
> + recording::type* t = m_types[type_id];
> + if (reinterpret_cast<intptr_t>(t) == -1)
> + return nullptr;
> + return t;
> +}
> +
> /* Create the recording::type for a given type of builtin function. */
>
> recording::type *
> @@ -661,15 +706,30 @@ tree
> builtins_manager::get_attrs_tree (enum built_in_function builtin_id)
> {
> enum built_in_attribute attr = builtin_data[builtin_id].attr;
> + if (attr == ATTR_LAST)
> + return get_attrs_tree_for_stub (builtin_id);
> return get_attrs_tree (attr);
> }
>
> -/* As above, but for an enum built_in_attribute. */
> +/* Get attributes for builtin stubs. */
> +
> +tree
> +builtins_manager::get_attrs_tree_for_stub (enum built_in_function builtin_id)
> +{
> + switch (builtin_id)
> + {
> + default:
> + return NULL_TREE;
> + case BUILT_IN_ALLOCA_WITH_ALIGN:
> + return get_attrs_tree (BUILT_IN_ALLOCA);
> + }
> +}
> +
> +/* As get_attrs_tree, but for an enum built_in_attribute. */
>
> tree
> builtins_manager::get_attrs_tree
> (enum built_in_attribute attr)
> {
> - gcc_assert (attr < ATTR_LAST);
> if (!m_attributes [attr])
> m_attributes [attr] = make_attrs_tree (attr);
> return m_attributes [attr];
> diff --git a/gcc/jit/jit-builtins.h b/gcc/jit/jit-builtins.h
> index 17e118481d6..f4de3707201 100644
> --- a/gcc/jit/jit-builtins.h
> +++ b/gcc/jit/jit-builtins.h
> @@ -124,6 +124,9 @@ public:
> tree
> get_attrs_tree (enum built_in_function builtin_id);
>
> + tree
> + get_attrs_tree_for_stub (enum built_in_function builtin_id);
> +
> tree
> get_attrs_tree (enum built_in_attribute attr);
>
> @@ -146,6 +149,10 @@ private:
> recording::type *
> make_type (enum jit_builtin_type type_id);
>
> + recording::type *get_type_for_stub (enum built_in_function type_id);
> +
> + recording::type *make_type_for_stub (enum built_in_function type_id);
> +
> recording::type*
> make_primitive_type (enum jit_builtin_type type_id);
>
> diff --git a/gcc/testsuite/jit.dg/test-aligned-alloca.c
> b/gcc/testsuite/jit.dg/t
> est-aligned-alloca.c
> new file mode 100644
> index 00000000000..e107746b82f
> --- /dev/null
> +++ b/gcc/testsuite/jit.dg/test-aligned-alloca.c
> @@ -0,0 +1,141 @@
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <stddef.h>
> +
> +#include "libgccjit.h"
> +
> +#include "harness.h"
> +
> +void
> +fill (void *ptr) {
> + for (int i = 0; i < 100; i++)
> + ((int *)ptr)[i] = i;
> +}
> +
> +void
> +sum (void *ptr, int *sum) {
> + *sum = 0;
> + for (int i = 0; i < 100; i++)
> + *sum += ((int *)ptr)[i];
> +}
> +
> +
> +void
> +create_code (gcc_jit_context *ctxt, void *user_data)
> +{
> + /*
> + bool test_aligned_alloca (typeof(fill) *fill, typeof(sum) *sum, int*
> sum_p) {
> + size_t addr;
> + void *p;
> +
> + p = __builtin_alloca_with_align (sizeof (int) * 100, 128);
> + addr = (size_t)p;
> + fill (p);
> + sum (p, sum_p);
> + return (addr & 127) == 0;
> + }
> + */
> +
> + /* Types */
> + gcc_jit_type *size_t_type
> + = gcc_jit_context_g
> et_type (ctxt, GCC_JIT_TYPE_SIZE_T);
> + gcc_jit_type *int_type
> + = gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
> + gcc_jit_type *int_ptr_type
> + = gcc_jit_type_get_pointer (int_type);
> + gcc_jit_type *void_ptr_type
> + = gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID_PTR);
> + gcc_jit_type *bool_type
> + = gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_BOOL);
> + gcc_jit_type *void_type
> + = gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID);
> + gcc_jit_type *fill_ptr_type
> + = gcc_jit_context_new_function_ptr_type (ctxt, NULL, void_type, 1,
> &void_ptr_type, 0);
> + gcc_jit_type *sum_params[] = {void_ptr_type, int_ptr_type};
> + gcc_jit_type *sum_ptr_type
> + = gcc_jit_context_new_function_ptr_type (ctxt, NULL, void_type, 2,
> sum_params, 0);
> +
> + /* Function */
> + gcc_jit_param *fill_param
> + = gcc_jit_context_new_param (ctxt, NULL, fill_ptr_type, "fill");
> + gcc_jit_rvalue *rv_fill = gcc_jit_pa
> ram_as_rvalue (fill_param);
> + gcc_jit_param *sum_param
> + = gcc_jit_context_new_param (ctxt, NULL, sum_ptr_type, "sum");
> + gcc_jit_rvalue *rv_sum = gcc_jit_param_as_rvalue (sum_param);
> + gcc_jit_param *sum_p_param
> + = gcc_jit_context_new_param (ctxt, NULL, int_ptr_type, "sum_p");
> + gcc_jit_rvalue *rv_sum_p = gcc_jit_param_as_rvalue (sum_p_param);
> + gcc_jit_param *params[] = {fill_param, sum_param, sum_p_param};
> + gcc_jit_function *test_aligned_alloca
> + = gcc_jit_context_new_function (ctxt, NULL,
> GCC_JIT_FUNCTION_EXPORTED, bool_type, "test_aligned_alloca", 3, params, 0);
> +
> +
> + /* Variables */
> + gcc_jit_lvalue *addr
> + = gcc_jit_function_new_local (test_aligned_alloca, NULL,
> size_t_type, "addr");
> + gcc_jit_lvalue *p
> + = gcc_jit_function_new_local (test_aligned_alloca, NULL,
> void_ptr_type, "p");
> + gcc_jit_rvalue *rv_addr = gcc_jit_lvalue_as_rvalue (addr);
> + gcc_jit_rvalue *rv_p = gcc_jit_lvalue_as_rv
> alue (p);
> +
> +
> + /* Blocks */
> + gcc_jit_block *block
> + = gcc_jit_function_new_block (test_aligned_alloca, NULL);
> +
> + /* p = __builtin_alloca_with_align (sizeof (int) * 100, 128); */
> + gcc_jit_rvalue *sizeof_int
> + = gcc_jit_context_new_sizeof(ctxt, int_type);
> + gcc_jit_rvalue *c100
> + = gcc_jit_context_new_rvalue_from_int(ctxt, int_type, 100);
> + gcc_jit_rvalue *size
> + = gcc_jit_context_new_binary_op(ctxt, NULL, GCC_JIT_BINARY_OP_MULT,
> size_t_type, sizeof_int, c100);
> + gcc_jit_rvalue *c128
> + = gcc_jit_context_new_rvalue_from_int(ctxt, size_t_type, 128);
> + gcc_jit_function *alloca_with_align
> + = gcc_jit_context_get_builtin_function (ctxt,
> "__builtin_alloca_with_align");
> + gcc_jit_rvalue *args[] = {size, c128};
> + gcc_jit_rvalue *alloca_with_align_call
> + = gcc_jit_context_new_call (ctxt, NULL, alloca_with_align, 2, args);
> + gcc_jit_block_add_assignment (block, NULL, p, alloca_with_align_
> call);
> +
> + /* addr = (size_t)p; */
> + gcc_jit_rvalue *cast_p
> + = gcc_jit_context_new_bitcast (ctxt, NULL, rv_p, size_t_type);
> + gcc_jit_block_add_assignment (block, NULL, addr, cast_p);
> +
> + /* fill (p); */
> + gcc_jit_rvalue * call_fill = gcc_jit_context_new_call_through_ptr(
> + ctxt, NULL, rv_fill, 1, &rv_p);
> + gcc_jit_block_add_eval (block, NULL, call_fill);
> +
> + /* sum (p, sum_p); */
> + gcc_jit_rvalue * sum_args[] = {rv_p, rv_sum_p};
> + gcc_jit_rvalue * call_sum = gcc_jit_context_new_call_through_ptr(
> + ctxt, NULL, rv_sum, 2, sum_args);
> + gcc_jit_block_add_eval (block, NULL, call_sum);
> +
> + /* return (addr & 127) == 0; */
> + gcc_jit_rvalue *c127
> + = gcc_jit_context_new_rvalue_from_int(ctxt, size_t_type, 127);
> + gcc_jit_rvalue *and_op
> + = gcc_jit_context_new_binary_op(ctxt, NULL,
> GCC_JIT_BINARY_OP_BITWISE_AND, size_t_type, rv_addr, c127);
> + gcc_jit_rvalue *c0
> + = gcc_jit_context_new_rva
> lue_from_int(ctxt, size_t_type, 0);
> + gcc_jit_rvalue *eq_op
> + = gcc_jit_context_new_comparison(ctxt, NULL, GCC_JIT_COMPARISON_EQ,
> and_op, c0);
> + gcc_jit_block_end_with_return (block, NULL, eq_op);
> +}
> +
> +void
> +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
> +{
> + typedef void (*fn_type) (typeof(fill) *, typeof(sum) *, int *);
> + CHECK_NON_NULL (result);
> + fn_type test_aligned_alloca =
> + (fn_type)gcc_jit_result_get_code (result, "test_aligned_alloca");
> + CHECK_NON_NULL (test_aligned_alloca);
> + int value;
> + test_aligned_alloca (fill, sum, &value);
> + CHECK (value == 4950);
> +}
> diff --git a/gcc/testsuite/jit.dg/test-stack-save-restore.c
> b/gcc/testsuite/jit.dg/test-stack-save-restore.c
> new file mode 100644
> index 00000000000..0bee247be9f
> --- /dev/null
> +++ b/gcc/testsuite/jit.dg/test-stack-save-restore.c
> @@ -0,0 +1,116 @@
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <stddef.h>
> +
> +#include "libgccjit.h"
> +
> +#include "harness.h"
>
> +
> +void
> +create_code (gcc_jit_context *ctxt, void *user_data)
> +{
> + /*
> + size_t test_stack_save_restore() {
> + void *p;
> + size_t a, b;
> + p = __builtin_stack_save();
> + a = (size_t)__builtin_alloca(1024);
> + __builtin_stack_restore(p);
> +
> + p = __builtin_stack_save();
> + b = (size_t)__builtin_alloca(512);
> + __builtin_stack_restore(p);
> +
> + return b - a;
> + }
> + */
> +
> + /* Types */
> + gcc_jit_type *size_t_type
> + = gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_SIZE_T);
> + gcc_jit_type *void_ptr_type
> + = gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID_PTR);
> +
> + /* Function */
> + gcc_jit_function *test_stack_save_restore
> + = gcc_jit_context_new_function (ctxt, NULL,
> GCC_JIT_FUNCTION_EXPORTED, size_t_type, "test_stack_save_restore", 0, NULL,
> 0);
> +
> +
> + /* Variables */
> + gcc_jit_lvalue *p
> + = gcc_jit_function_new_local (test_stack_save_restore, NULL,
> void_ptr_type, "p");
> + gcc_jit_lv
> alue *a
> + = gcc_jit_function_new_local (test_stack_save_restore, NULL,
> size_t_type, "a");
> + gcc_jit_lvalue *b
> + = gcc_jit_function_new_local (test_stack_save_restore, NULL,
> size_t_type, "b");
> + gcc_jit_rvalue *rv_p = gcc_jit_lvalue_as_rvalue (p);
> + gcc_jit_rvalue *rv_a = gcc_jit_lvalue_as_rvalue (a);
> + gcc_jit_rvalue *rv_b = gcc_jit_lvalue_as_rvalue (b);
> +
> +
> + /* Blocks */
> + gcc_jit_block *block
> + = gcc_jit_function_new_block (test_stack_save_restore, NULL);
> +
> + /* Builtin functions */
> + gcc_jit_function *stack_save
> + = gcc_jit_context_get_builtin_function(ctxt, "__builtin_stack_save");
> + gcc_jit_function *stack_restore
> + = gcc_jit_context_get_builtin_function(ctxt,
> "__builtin_stack_restore");
> + gcc_jit_function *alloca
> + = gcc_jit_context_get_builtin_function(ctxt, "__builtin_alloca");
> +
> + /* Common code */
> + gcc_jit_rvalue *call_stack_save
> + = gcc_jit_context_new_call(ctxt, NULL, stack_save, 0, NULL);
> + gcc_jit_rvalue *
> call_stack_restore
> + = gcc_jit_context_new_call(ctxt, NULL, stack_restore, 1, &rv_p);
> +
> +
> + /* p = __builtin_stack_save(); */
> + gcc_jit_block_add_assignment (block, NULL, p, call_stack_save);
> +
> + /* a = (size_t)__builtin_alloca(1024); */
> + gcc_jit_rvalue *c1024
> + = gcc_jit_context_new_rvalue_from_int (ctxt, size_t_type, 1024);
> + gcc_jit_rvalue *call_alloca_1024
> + = gcc_jit_context_new_call(ctxt, NULL, alloca, 1, &c1024);
> + gcc_jit_rvalue *cast_alloca_1024
> + = gcc_jit_context_new_bitcast(ctxt, NULL, call_alloca_1024,
> size_t_type);
> + gcc_jit_block_add_assignment (block, NULL, a, cast_alloca_1024);
> +
> + /* __builtin_stack_restore(p); */
> + gcc_jit_block_add_eval (block, NULL, call_stack_restore);
> +
> + /* p = __builtin_stack_save(); */
> + gcc_jit_block_add_assignment (block, NULL, p, call_stack_save);
> +
> + /* b = (size_t)__builtin_alloca(512); */
> + gcc_jit_rvalue *c512
> + = gcc_jit_context_new_rvalue_from_int (ctxt, size_t_type, 512);
> + gcc_jit_rv
> alue *call_alloca_512
> + = gcc_jit_context_new_call(ctxt, NULL, alloca, 1, &c512);
> + gcc_jit_rvalue *cast_alloca_512
> + = gcc_jit_context_new_bitcast(ctxt, NULL, call_alloca_512,
> size_t_type);
> + gcc_jit_block_add_assignment (block, NULL, b, cast_alloca_512);
> +
> + /* __builtin_stack_restore(p); */
> + gcc_jit_block_add_eval (block, NULL, call_stack_restore);
> +
> + /* return b - a; */
> + gcc_jit_rvalue *sub
> + = gcc_jit_context_new_binary_op(ctxt, NULL, GCC_JIT_BINARY_OP_MINUS,
> size_t_type, rv_b, rv_a);
> + gcc_jit_block_end_with_return (block, NULL, sub);
> +}
> +
> +void
> +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
> +{
> + typedef size_t (*fn_type) (void);
> + CHECK_NON_NULL (result);
> + fn_type test_stack_save_restore =
> + (fn_type)gcc_jit_result_get_code (result, "test_stack_save_restore");
> + CHECK_NON_NULL (test_stack_save_restore);
> + size_t value = test_stack_save_restore();
> + CHECK (value == 512);
> +}
> diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tre
> e-ssa-ccp.cc
> index fcb91c58335..d721c471f74 100644
> --- a/gcc/tree-ssa-ccp.cc
> +++ b/gcc/tree-ssa-ccp.cc
> @@ -2671,7 +2671,8 @@ fold_builtin_alloca_with_align (gimple *stmt)
> block = gimple_block (stmt);
> if (!(cfun->after_inlining
> && block
> - && TREE_CODE (BLOCK_SUPERCONTEXT (block)) == FUNCTION_DECL))
> + && BLOCK_SUPERCONTEXT (block)
> + && TREE_CODE (BLOCK_SUPERCONTEXT (block)) == FUNCTION_DECL))
This change seems incorrect. Every block should have a super context.
Including the last block which should point to a function decl. Maybe
gccjit is not setting up the blocks correctly where the supper most
one is not setting its context to being the function decl.
I think the following patch will fix that bug:
```
diff --git a/gcc/jit/jit-playback.cc b/gcc/jit/jit-playback.cc
index e32e837f2fe..acdb25c7d09 100644
--- a/gcc/jit/jit-playback.cc
+++ b/gcc/jit/jit-playback.cc
@@ -2118,6 +2118,7 @@ postprocess ()
/* Seem to need this in gimple-low.cc: */
gcc_assert (m_inner_block);
DECL_INITIAL (m_inner_fndecl) = m_inner_block;
+ BLOCK_SUPERCONTEXT (m_inner_block) = m_inner_fndecl;
/* how to add to function? the following appears to be how to
set the body of a m_inner_fndecl: */
```
Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
Thanks,
Andrew Pinski
> threshold /= 10;
> if (size > threshold)
> return NULL_TREE;
> --
> 2.43.0
>
