[GCCJIT] support dynamic alloca stub

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

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_apinski@quicinc.com>


Thanks,
Andrew Pinski


>      threshold /= 10;
>    if (size > threshold)
>      return NULL_TREE;
> --
> 2.43.0
>

Thank you for the quick review. Indeed, I reverted the changes to tree-ssa-ccp.cc and applied your changes. All tests still pass.



Schrodinger ZHU Yifan, Ph.D. Student
Computer Science Department, University of Rochester

Personal Email: i@zhuyi.fan
Work Email: yifanzhu@rochester.edu
Website: https://www.cs.rochester.edu/~yzhu104/Main.html
Github: SchrodingerZhu
GPG Fingerprint: BA02CBEB8CB5D8181E9368304D2CC545A78DBCC3


On Saturday, November 9th, 2024 at 11:47 PM, Andrew Pinski <pinskia@gmail.com> wrote:

> 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_apinski@quicinc.com
> 

> 

> 

> Thanks,
> Andrew Pinski
> 

> > threshold /= 10;
> > if (size > threshold)
> > return NULL_TREE;
> > --
> > 2.43.0