[PATCH] [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
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(-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(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);
re
Re: [PATCH] [GCCJIT] support dynamic alloca stub
On Sat, Nov 9, 2024 at 8:30 PM Schrodinger ZHU Yifan 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(-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(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:
> +
Re: [PATCH] [GCCJIT] support dynamic alloca stub
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
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(-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(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
