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: yifan...@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 <pins...@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_apin...@quicinc.com > > > > Thanks, > Andrew Pinski > > > threshold /= 10; > > if (size > threshold) > > return NULL_TREE; > > -- > > 2.43.0
publickey - i@zhuyi.fan - 0xA98A3EAE.asc
Description: application/pgp-keys
signature.asc
Description: OpenPGP digital signature
