| Message ID | gkrd0jvvvx2.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | libgccjit: add bitfield support | expand |
Just wanted to politely ping to have have a status on that. Bests Andrea Andrea Corallo writes: > Hi all, > I would like to submit this patch that aims to introduce bitfields support into libgccjit. > > A new entry point gcc_jit_context_new_bitfield is added plus relative testcase. > > Checked with make check-jit does not introduce regressions. > > Feedbacks are very welcome. > > Bests > > Andrea > > 2019-06-01 Andrea Corallo andrea.corallo@arm.com > > * docs/topics/compatibility.rst (LIBGCCJIT_ABI_12): New ABI tag. > * docs/topics/types.rst: Add gcc_jit_context_new_bitfield. > * jit-common.h (namespace recording): Add class bitfield. > * jit-playback.c: Include "c-family/c-common.h" > (playback::context::new_bitfield): New method. > (playback::compound_type::set_fields): Add bitfield support. > (playback::lvalue::jit_mark_addressable): Make this a method of lvalue > plus return a bool to communicate success. > (playback::lvalue::get_address): Check for jit_mark_addressable return > value. > * jit-playback.h (new_bitfield): New method. > (class bitfield): New class. > (class lvalue): Add jit_mark_addressable method. > * jit-recording.c (recording::context::new_bitfield): New method. > (recording::bitfield::replay_into): New method. > (recording::bitfield::write_to_dump): Likewise. > (recording::bitfield::make_debug_string): Likewise. > (recording::bitfield::write_reproducer): Likewise. > * jit-recording.h (class context): Add new_bitfield method. > (class field): Make it derivable by class bitfield. > (class bitfield): Add new class. > * libgccjit++.h (class context): Add new_bitfield method. > * libgccjit.c (struct gcc_jit_bitfield): New structure. > (gcc_jit_context_new_bitfield): New function. > * libgccjit.h > (LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield) New macro. > (gcc_jit_context_new_bitfield): New function. > * libgccjit.map (LIBGCCJIT_ABI_12) New ABI tag. > > > 2019-06-01 Andrea Corallo andrea.corallo@arm.com > > * jit.dg/all-non-failing-tests.h: Add test-accessing-bitfield.c. > * jit.dg/test-accessing-bitfield.c: New testcase. > diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst > index abefa56..da64920 100644 > --- a/gcc/jit/docs/topics/compatibility.rst > +++ b/gcc/jit/docs/topics/compatibility.rst > @@ -177,3 +177,8 @@ entrypoints: > -------------------- > ``LIBGCCJIT_ABI_11`` covers the addition of > :func:`gcc_jit_context_add_driver_option` > + > +``LIBGCCJIT_ABI_12`` > +-------------------- > +``LIBGCCJIT_ABI_12`` covers the addition of > +:func:`gcc_jit_context_new_bitfield` > diff --git a/gcc/jit/docs/topics/types.rst b/gcc/jit/docs/topics/types.rst > index 1d2dcd4..094f9fd 100644 > --- a/gcc/jit/docs/topics/types.rst > +++ b/gcc/jit/docs/topics/types.rst > @@ -247,6 +247,30 @@ You can model C `struct` types by creating :c:type:`gcc_jit_struct *` and > underlying string, so it is valid to pass in a pointer to an on-stack > buffer. > > +.. function:: gcc_jit_field *\ > + gcc_jit_context_new_bitfield (gcc_jit_context *ctxt,\ > + gcc_jit_location *loc,\ > + gcc_jit_type *type,\ > + int width,\ > + const char *name) > + > + Construct a new bit field, with the given type width and name. > + > + The parameter ``name`` must be non-NULL. The call takes a copy of the > + underlying string, so it is valid to pass in a pointer to an on-stack > + buffer. > + > + The parameter ``type`` must be an integer type. > + > + The parameter ``width`` must be a positive integer that does not exceed the > + size of ``type``. > + > + This API entrypoint was added in :ref:`LIBGCCJIT_ABI_12`; you can test > + for its presence using > + .. code-block:: c > + > + #ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield > + > .. function:: gcc_jit_object *\ > gcc_jit_field_as_object (gcc_jit_field *field) > > diff --git a/gcc/jit/jit-common.h b/gcc/jit/jit-common.h > index 1d96cc3..e747d96 100644 > --- a/gcc/jit/jit-common.h > +++ b/gcc/jit/jit-common.h > @@ -119,6 +119,7 @@ namespace recording { > class union_; > class vector_type; > class field; > + class bitfield; > class fields; > class function; > class block; > diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c > index b74495c..7676e55 100644 > --- a/gcc/jit/jit-playback.c > +++ b/gcc/jit/jit-playback.c > @@ -31,6 +31,7 @@ along with GCC; see the file COPYING3. If not see > #include "convert.h" > #include "stor-layout.h" > #include "print-tree.h" > +#include "c-family/c-common.h" > #include "gimplify.h" > #include "gcc-driver-name.h" > #include "attribs.h" > @@ -263,6 +264,48 @@ new_field (location *loc, > return new field (decl); > } > > +/* Construct a playback::bitfield instance (wrapping a tree). */ > + > +playback::field * > +playback::context:: > +new_bitfield (location *loc, > + type *type, > + int width, > + const char *name) > +{ > + gcc_assert (type); > + gcc_assert (name); > + gcc_assert (width); > + > + /* compare with c/c-decl.c:grokfield, grokdeclarator and > + check_bitfield_type_and_width. */ > + > + tree tree_type = type->as_tree (); > + if (TREE_CODE (tree_type) != INTEGER_TYPE > + && TREE_CODE (tree_type) != BOOLEAN_TYPE) > + { > + add_error (loc, "bit-field %s has invalid type", name); > + return NULL; > + } > + tree tree_width = build_int_cst (integer_type_node, width); > + if (compare_tree_int (tree_width, TYPE_PRECISION (tree_type)) > 0) > + { > + add_error (loc, "bit-field %s exceeds its type", name); > + return NULL; > + } > + > + tree decl = build_decl (UNKNOWN_LOCATION, FIELD_DECL, > + get_identifier (name), type->as_tree ()); > + DECL_NONADDRESSABLE_P (decl) = true; > + DECL_INITIAL (decl) = tree_width; > + SET_DECL_C_BIT_FIELD (decl); > + > + if (loc) > + set_tree_location (decl, loc); > + > + return new field (decl); > +} > + > /* Construct a playback::compound_type instance (wrapping a tree). */ > > playback::compound_type * > @@ -295,8 +338,15 @@ playback::compound_type::set_fields (const auto_vec<playback::field *> *fields) > for (unsigned i = 0; i < fields->length (); i++) > { > field *f = (*fields)[i]; > - DECL_CONTEXT (f->as_tree ()) = t; > - fieldlist = chainon (f->as_tree (), fieldlist); > + tree x = f->as_tree (); > + DECL_CONTEXT (x) = t; > + if (DECL_C_BIT_FIELD (x)) > + { > + unsigned HOST_WIDE_INT width = tree_to_uhwi (DECL_INITIAL (x)); > + DECL_SIZE (x) = bitsize_int (width); > + DECL_BIT_FIELD (x) = 1; > + } > + fieldlist = chainon (x, fieldlist); > } > fieldlist = nreverse (fieldlist); > TYPE_FIELDS (t) = fieldlist; > @@ -1197,20 +1247,30 @@ dereference (location *loc) > return new lvalue (get_context (), datum); > } > > -/* Mark EXP saying that we need to be able to take the > +/* Mark the lvalue saying that we need to be able to take the > address of it; it should not be allocated in a register. > - Compare with e.g. c/c-typeck.c: c_mark_addressable. */ > + Compare with e.g. c/c-typeck.c: c_mark_addressable really_atomic_lvalue. > + Returns true if successful. */ > > -static void > -jit_mark_addressable (tree exp) > +bool > +playback::lvalue:: > +jit_mark_addressable (location *loc) > { > - tree x = exp; > + tree x = as_tree ();; > > while (1) > switch (TREE_CODE (x)) > { > case COMPONENT_REF: > - /* (we don't yet support bitfields) */ > + if (DECL_C_BIT_FIELD (TREE_OPERAND (x, 1))) > + { > + gcc_assert (gcc::jit::active_playback_ctxt); > + gcc::jit:: > + active_playback_ctxt->add_error (loc, > + "cannot take address of " > + "bit-field"); > + return false; > + } > /* fallthrough */ > case ADDR_EXPR: > case ARRAY_REF: > @@ -1222,7 +1282,7 @@ jit_mark_addressable (tree exp) > case COMPOUND_LITERAL_EXPR: > case CONSTRUCTOR: > TREE_ADDRESSABLE (x) = 1; > - return; > + return true; > > case VAR_DECL: > case CONST_DECL: > @@ -1234,7 +1294,7 @@ jit_mark_addressable (tree exp) > TREE_ADDRESSABLE (x) = 1; > /* fallthrough */ > default: > - return; > + return true; > } > } > > @@ -1251,8 +1311,10 @@ get_address (location *loc) > tree ptr = build1 (ADDR_EXPR, t_ptrtype, t_lvalue); > if (loc) > get_context ()->set_tree_location (ptr, loc); > - jit_mark_addressable (t_lvalue); > - return new rvalue (get_context (), ptr); > + if (jit_mark_addressable (loc)) > + return new rvalue (get_context (), ptr); > + else > + return NULL; > } > > /* The wrapper subclasses are GC-managed, but can own non-GC memory. > diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h > index bc4de9c..c59f614 100644 > --- a/gcc/jit/jit-playback.h > +++ b/gcc/jit/jit-playback.h > @@ -75,6 +75,12 @@ public: > type *type, > const char *name); > > + field * > + new_bitfield (location *loc, > + type *type, > + int width, > + const char *name); > + > compound_type * > new_compound_type (location *loc, > const char *name, > @@ -426,6 +432,8 @@ private: > tree m_inner; > }; > > +class bitfield : public field {}; > + > class function : public wrapper > { > public: > @@ -614,6 +622,8 @@ public: > rvalue * > get_address (location *loc); > > +private: > + bool jit_mark_addressable (location *loc); > }; > > class param : public lvalue > @@ -703,4 +713,3 @@ extern playback::context *active_playback_ctxt; > } // namespace gcc > > #endif /* JIT_PLAYBACK_H */ > - > diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c > index a332fe8..0678d07 100644 > --- a/gcc/jit/jit-recording.c > +++ b/gcc/jit/jit-recording.c > @@ -872,6 +872,24 @@ recording::context::new_field (recording::location *loc, > return result; > } > > +/* Create a recording::bitfield instance and add it to this context's list > + of mementos. > + > + Implements the post-error-checking part of > + gcc_jit_context_new_bitfield. */ > + > +recording::field * > +recording::context::new_bitfield (recording::location *loc, > + recording::type *type, > + int width, > + const char *name) > +{ > + recording::field *result = > + new recording::bitfield (this, loc, type, width, new_string (name)); > + record (result); > + return result; > +} > + > /* Create a recording::struct_ instance and add it to this context's > list of mementos and list of compound types. > > @@ -2999,6 +3017,66 @@ recording::field::write_reproducer (reproducer &r) > m_name->get_debug_string ()); > } > > +/* The implementation of class gcc::jit::recording::bitfield. */ > + > +/* Implementation of pure virtual hook recording::memento::replay_into > + for recording::bitfield. */ > + > +void > +recording::bitfield::replay_into (replayer *r) > +{ > + set_playback_obj (r->new_bitfield (playback_location (r, m_loc), > + m_type->playback_type (), > + m_width, > + playback_string (m_name))); > +} > + > +/* Override the default implementation of > + recording::memento::write_to_dump. Dump each bit field > + by dumping a line of the form: > + TYPE NAME:WIDTH; > + so that we can build up a struct/union field-byfield. */ > + > +void > +recording::bitfield::write_to_dump (dump &d) > +{ > + d.write (" %s %s:%d;\n", > + m_type->get_debug_string (), > + m_name->c_str (), > + m_width); > +} > + > +/* Implementation of recording::memento::make_debug_string for > + results of new_bitfield. */ > + > +recording::string * > +recording::bitfield::make_debug_string () > +{ > + return string::from_printf (m_ctxt, > + "%s:%d", > + m_name->c_str (), m_width); > +} > + > +/* Implementation of recording::memento::write_reproducer for bitfields. */ > + > +void > +recording::bitfield::write_reproducer (reproducer &r) > +{ > + const char *id = r.make_identifier (this, "bitfield"); > + r.write (" gcc_jit_field *%s =\n" > + " gcc_jit_context_new_bitfield (%s,\n" > + " %s, /* gcc_jit_location *loc */\n" > + " %s, /* gcc_jit_type *type, */\n" > + " %d, /* int *width, */\n" > + " %s); /* const char *name */\n", > + id, > + r.get_identifier (get_context ()), > + r.get_identifier (m_loc), > + r.get_identifier_as_type (m_type), > + m_width, > + m_name->get_debug_string ()); > +} > + > /* The implementation of class gcc::jit::recording::compound_type */ > > /* The constructor for gcc::jit::recording::compound_type. */ > diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h > index b9f2250..5e2d64e 100644 > --- a/gcc/jit/jit-recording.h > +++ b/gcc/jit/jit-recording.h > @@ -95,6 +95,12 @@ public: > type *type, > const char *name); > > + field * > + new_bitfield (location *loc, > + type *type, > + int width, > + const char *name); > + > struct_ * > new_struct_type (location *loc, > const char *name); > @@ -822,9 +828,9 @@ public: > compound_type * get_container () const { return m_container; } > void set_container (compound_type *c) { m_container = c; } > > - void replay_into (replayer *) FINAL OVERRIDE; > + void replay_into (replayer *); > > - void write_to_dump (dump &d) FINAL OVERRIDE; > + void write_to_dump (dump &d); > > playback::field * > playback_field () const > @@ -833,16 +839,40 @@ public: > } > > private: > - string * make_debug_string () FINAL OVERRIDE; > - void write_reproducer (reproducer &r) FINAL OVERRIDE; > + string * make_debug_string (); > + void write_reproducer (reproducer &r); > > -private: > +protected: > location *m_loc; > type *m_type; > string *m_name; > compound_type *m_container; > }; > > + > +class bitfield : public field > +{ > +public: > + bitfield (context *ctxt, > + location *loc, > + type *type, > + int width, > + string *name) > + : field (ctxt, loc, type, name) > + { m_width = width; } > + > + void replay_into (replayer *) FINAL OVERRIDE; > + > + void write_to_dump (dump &d) FINAL OVERRIDE; > + > +private: > + string * make_debug_string () FINAL OVERRIDE; > + void write_reproducer (reproducer &r) FINAL OVERRIDE; > + > +private: > + int m_width; > +}; > + > /* Base class for struct_ and union_ */ > class compound_type : public type > { > diff --git a/gcc/jit/libgccjit++.h b/gcc/jit/libgccjit++.h > index 55aebca..e5a769a 100644 > --- a/gcc/jit/libgccjit++.h > +++ b/gcc/jit/libgccjit++.h > @@ -152,6 +152,9 @@ namespace gccjit > field new_field (type type_, const std::string &name, > location loc = location ()); > > + field new_bitfield (type type_, int width, const std::string &name, > + location loc = location ()); > + > struct_ new_struct_type (const std::string &name, > std::vector<field> &fields, > location loc = location ()); > @@ -757,6 +760,17 @@ context::new_field (type type_, const std::string &name, location loc) > name.c_str ())); > } > > +inline field > +context::new_bitfield (type type_, int width, const std::string &name, > + location loc) > +{ > + return field (gcc_jit_context_new_bitfield (m_inner_ctxt, > + loc.get_inner_location (), > + type_.get_inner_type (), > + width, > + name.c_str ())); > +} > + > inline struct_ > context::new_struct_type (const std::string &name, > std::vector<field> &fields, > diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c > index e4f17f8..f1265a5 100644 > --- a/gcc/jit/libgccjit.c > +++ b/gcc/jit/libgccjit.c > @@ -62,6 +62,10 @@ struct gcc_jit_field : public gcc::jit::recording::field > { > }; > > +struct gcc_jit_bitfield : public gcc::jit::recording::bitfield > +{ > +}; > + > struct gcc_jit_function : public gcc::jit::recording::function > { > }; > @@ -556,6 +560,35 @@ gcc_jit_context_new_field (gcc_jit_context *ctxt, > > /* Public entrypoint. See description in libgccjit.h. > > + After error-checking, the real work is done by the > + gcc::jit::recording::context::new_bitfield method, in > + jit-recording.c. */ > + > +gcc_jit_field * > +gcc_jit_context_new_bitfield (gcc_jit_context *ctxt, > + gcc_jit_location *loc, > + gcc_jit_type *type, > + int width, > + const char *name) > +{ > + RETURN_NULL_IF_FAIL (ctxt, NULL, NULL, "NULL context"); > + JIT_LOG_FUNC (ctxt->get_logger ()); > + /* LOC can be NULL. */ > + RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type"); > + RETURN_NULL_IF_FAIL (width > 0, ctxt, NULL, "invalid width"); > + RETURN_NULL_IF_FAIL (name, ctxt, loc, "NULL name"); > + RETURN_NULL_IF_FAIL_PRINTF2 ( > + type->has_known_size (), > + ctxt, loc, > + "unknown size for field \"%s\" (type: %s)", > + name, > + type->get_debug_string ()); > + > + return (gcc_jit_field *)ctxt->new_bitfield (loc, type, width, name); > +} > + > +/* Public entrypoint. See description in libgccjit.h. > + > After error-checking, this calls the trivial > gcc::jit::recording::memento::as_object method (a field is a > memento), in jit-recording.h. */ > diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h > index beeb747..9c5f23b 100644 > --- a/gcc/jit/libgccjit.h > +++ b/gcc/jit/libgccjit.h > @@ -602,6 +602,21 @@ gcc_jit_context_new_field (gcc_jit_context *ctxt, > gcc_jit_type *type, > const char *name); > > +#define LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield > + > +/* Create a bit field, for use within a struct or union. > + > + This API entrypoint was added in LIBGCCJIT_ABI_12; you can test for its > + presence using > + #ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield > +*/ > +extern gcc_jit_field * > +gcc_jit_context_new_bitfield (gcc_jit_context *ctxt, > + gcc_jit_location *loc, > + gcc_jit_type *type, > + int width, > + const char *name); > + > /* Upcasting from field to object. */ > extern gcc_jit_object * > gcc_jit_field_as_object (gcc_jit_field *field); > diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map > index 16f5253..40e1c78 100644 > --- a/gcc/jit/libgccjit.map > +++ b/gcc/jit/libgccjit.map > @@ -174,4 +174,9 @@ LIBGCCJIT_ABI_10 { > LIBGCCJIT_ABI_11 { > global: > gcc_jit_context_add_driver_option; > -} LIBGCCJIT_ABI_10; > \ No newline at end of file > +} LIBGCCJIT_ABI_10; > + > +LIBGCCJIT_ABI_12 { > + global: > + gcc_jit_context_new_bitfield; > +} LIBGCCJIT_ABI_11; > \ No newline at end of file > diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h > index 9a10418..f7af8e9 100644 > --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h > +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h > @@ -8,6 +8,13 @@ > hooks provided by each test case. */ > #define COMBINED_TEST > > +/* test-accessing-bitfield.c */ > +#define create_code create_code_accessing_bitfield > +#define verify_code verify_code_accessing_bitfield > +#include "test-accessing-bitfield.c" > +#undef create_code > +#undef verify_code > + > /* test-accessing-struct.c */ > #define create_code create_code_accessing_struct > #define verify_code verify_code_accessing_struct > diff --git a/gcc/testsuite/jit.dg/test-accessing-bitfield.c b/gcc/testsuite/jit.dg/test-accessing-bitfield.c > new file mode 100644 > index 0000000..6ca8b24 > --- /dev/null > +++ b/gcc/testsuite/jit.dg/test-accessing-bitfield.c > @@ -0,0 +1,130 @@ > +#include <stdlib.h> > +#include <stdio.h> > + > +#include "libgccjit.h" > + > +#include "harness.h" > + > +struct bit_foo > +{ > + int i:3; > + int x:5; > + int y:5; > + int z:10; > + int j:3; > +}; > + > +void > +create_code (gcc_jit_context *ctxt, void *user_data) > +{ > + /* Let's try to inject the equivalent of: > + void > + test_access (struct bit_foo *f) > + { > + f->z = f->x + f->y; > + } > + */ > + gcc_jit_type *void_type = > + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID); > + gcc_jit_type *int_type = > + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT); > + gcc_jit_field *i = > + gcc_jit_context_new_bitfield (ctxt, > + NULL, > + int_type, > + 3, > + "i"); > + gcc_jit_field *x = > + gcc_jit_context_new_bitfield (ctxt, > + NULL, > + int_type, > + 5, > + "x"); > + gcc_jit_field *y = > + gcc_jit_context_new_bitfield (ctxt, > + NULL, > + int_type, > + 5, > + "y"); > + gcc_jit_field *z = > + gcc_jit_context_new_bitfield (ctxt, > + NULL, > + int_type, > + 10, > + "z"); > + gcc_jit_field *j = > + gcc_jit_context_new_bitfield (ctxt, > + NULL, > + int_type, > + 3, > + "j"); > + gcc_jit_field *fields[] = {i, x, y, z, j}; > + gcc_jit_struct *struct_type = > + gcc_jit_context_new_struct_type (ctxt, NULL, "bit_foo", 5, fields); > + gcc_jit_type *ptr_type = > + gcc_jit_type_get_pointer (gcc_jit_struct_as_type (struct_type)); > + > + /* Build the test function. */ > + gcc_jit_param *param_f = > + gcc_jit_context_new_param (ctxt, NULL, ptr_type, "f"); > + gcc_jit_function *test_fn = > + gcc_jit_context_new_function (ctxt, NULL, > + GCC_JIT_FUNCTION_EXPORTED, > + void_type, > + "test_access", > + 1, ¶m_f, > + 0); > + > + /* f->x + f->y */ > + gcc_jit_rvalue *sum = > + gcc_jit_context_new_binary_op ( > + ctxt, NULL, > + GCC_JIT_BINARY_OP_PLUS, > + int_type, > + gcc_jit_lvalue_as_rvalue ( > + gcc_jit_rvalue_dereference_field ( > + gcc_jit_param_as_rvalue (param_f), > + NULL, > + x)), > + gcc_jit_lvalue_as_rvalue ( > + gcc_jit_rvalue_dereference_field ( > + gcc_jit_param_as_rvalue (param_f), > + NULL, > + y))); > + > + /* f->z = ... */ > + gcc_jit_block *block = gcc_jit_function_new_block (test_fn, NULL); > + gcc_jit_block_add_assignment ( > + block, > + NULL, > + gcc_jit_rvalue_dereference_field ( > + gcc_jit_param_as_rvalue (param_f), > + NULL, > + z), > + sum); > + gcc_jit_block_end_with_void_return (block, NULL); > +} > + > +void > +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) > +{ > + typedef void (*fn_type) (struct bit_foo *); > + CHECK_NON_NULL (result); > + > + fn_type test_access = > + (fn_type)gcc_jit_result_get_code (result, "test_access"); > + CHECK_NON_NULL (test_access); > + > + struct bit_foo tmp; > + tmp.i = 3; > + tmp.x = 5; > + tmp.y = 7; > + tmp.z = 0; > + tmp.j = 3; > + > + /* Call the JIT-generated function. */ > + test_access (&tmp); > + > + /* Verify that the code correctly modified the field "z". */ > + CHECK_VALUE (tmp.z, 12); > +}
On Mon, 2019-06-03 at 09:51 +0000, Andrea Corallo wrote: > Hi all, > I would like to submit this patch that aims to introduce bitfields > support into libgccjit. > > A new entry point gcc_jit_context_new_bitfield is added plus relative > testcase. > > Checked with make check-jit does not introduce regressions. > > Feedbacks are very welcome. > > Bests > > Andrea > > 2019-06-01 Andrea Corallo andrea.corallo@arm.com > > * docs/topics/compatibility.rst (LIBGCCJIT_ABI_12): New ABI tag. > * docs/topics/types.rst: Add gcc_jit_context_new_bitfield. > * jit-common.h (namespace recording): Add class bitfield. > * jit-playback.c: Include "c-family/c-common.h" > (playback::context::new_bitfield): New method. > (playback::compound_type::set_fields): Add bitfield support. > (playback::lvalue::jit_mark_addressable): Make this a method of > lvalue > plus return a bool to communicate success. > (playback::lvalue::get_address): Check for jit_mark_addressable > return > value. > * jit-playback.h (new_bitfield): New method. > (class bitfield): New class. > (class lvalue): Add jit_mark_addressable method. > * jit-recording.c (recording::context::new_bitfield): New method. > (recording::bitfield::replay_into): New method. > (recording::bitfield::write_to_dump): Likewise. > (recording::bitfield::make_debug_string): Likewise. > (recording::bitfield::write_reproducer): Likewise. > * jit-recording.h (class context): Add new_bitfield method. > (class field): Make it derivable by class bitfield. > (class bitfield): Add new class. > * libgccjit++.h (class context): Add new_bitfield method. > * libgccjit.c (struct gcc_jit_bitfield): New structure. > (gcc_jit_context_new_bitfield): New function. > * libgccjit.h > (LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield) New macro. > (gcc_jit_context_new_bitfield): New function. > * libgccjit.map (LIBGCCJIT_ABI_12) New ABI tag. > > > 2019-06-01 Andrea Corallo andrea.corallo@arm.com > > * jit.dg/all-non-failing-tests.h: Add test-accessing-bitfield.c. > * jit.dg/test-accessing-bitfield.c: New testcase. Thanks for working on this; sorry for the delay in reviewing it. Overall, this looks close to being ready, but I have a few notes: [...] > diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c > index b74495c..7676e55 100644 > --- a/gcc/jit/jit-playback.c > +++ b/gcc/jit/jit-playback.c > @@ -31,6 +31,7 @@ along with GCC; see the file COPYING3. If not see > #include "convert.h" > #include "stor-layout.h" > #include "print-tree.h" > +#include "c-family/c-common.h" Presumably this is for DECL_C_BIT_FIELD etc, and this would make libgccjit piggy-back off of the C family's macros for using lang flag 4? If so, I think it would be cleaner to instead take a copy of those macros (at the top of jit-playback.c for now), renaming them to DECL_JIT_BIT_FIELD etc, with a "compare with" comment referring to the C frontend macros. That way libgccjit doesn't directly depend on implementation details of the C-family of frontends, but it's easy to see where we took the code from (I believe that libgccjit isn't currently making any use of lang flags, so this would be the first). > #include "gimplify.h" > #include "gcc-driver-name.h" > #include "attribs.h" > @@ -263,6 +264,48 @@ new_field (location *loc, > return new field (decl); > } > > +/* Construct a playback::bitfield instance (wrapping a tree). */ > + > +playback::field * > +playback::context:: > +new_bitfield (location *loc, > + type *type, > + int width, > + const char *name) > +{ > + gcc_assert (type); > + gcc_assert (name); > + gcc_assert (width); > + > + /* compare with c/c-decl.c:grokfield, grokdeclarator and > + check_bitfield_type_and_width. */ > + > + tree tree_type = type->as_tree (); > + if (TREE_CODE (tree_type) != INTEGER_TYPE > + && TREE_CODE (tree_type) != BOOLEAN_TYPE) > + { > + add_error (loc, "bit-field %s has invalid type", name); Ideally this error message would identify the type, and be more precise about what the problem with it is. I initially thought something like: add_error (loc, "bit-field %s has invalid type %s (must be integer or boolean)", name, type->get_debug_string ()); would work, but type is a playback::type, rather than a recording::type. Is there a way to catch this problem earlier, before we reach playback? Alternatively: add_error (loc, "bit-field %s has invalid type (must be integer or boolean)"); name); would at least be more precise about the problem. It would be good to have test coverage for this; see e.g. gcc/testsuite/jit.dg/test-error-gcc_jit_context_new_field-opaque-struct.c and similar, where testcases have the form: test-error-API_ENTRYPOINT-WHAT-WENT-WRONG.c > + return NULL; > + } > + tree tree_width = build_int_cst (integer_type_node, width); > + if (compare_tree_int (tree_width, TYPE_PRECISION (tree_type)) > 0) > + { > + add_error (loc, "bit-field %s exceeds its type", name); Similarly, could this message be reworded to: add_error (loc, "width of bit-field %s (width: %i) is wider than its type (width: %i)", name, width, something (TYPE_PRECISION (tree_type))); (using "something" as I can't remember the incantation to get from tree int cst with known small integer back to int) or could the problem be caught earlier? Again, would be good to have a testcase. (any non-trivial error-handling ideally ought to have a testcase). [...] > @@ -1197,20 +1247,30 @@ dereference (location *loc) > return new lvalue (get_context (), datum); > } > > -/* Mark EXP saying that we need to be able to take the > +/* Mark the lvalue saying that we need to be able to take the > address of it; it should not be allocated in a register. > - Compare with e.g. c/c-typeck.c: c_mark_addressable. */ > + Compare with e.g. c/c-typeck.c: c_mark_addressable really_atomic_lvalue. > + Returns true if successful. */ "Returns false if a failure occurred (an error will already have been added to the active context for this case)." (or somesuch). > > -static void > -jit_mark_addressable (tree exp) > +bool > +playback::lvalue:: > +jit_mark_addressable (location *loc) > { Given that this is becoming a member function, we can lose the "jit_" prefix, I think. > - tree x = exp; > + tree x = as_tree ();; > > while (1) > switch (TREE_CODE (x)) > { > case COMPONENT_REF: > - /* (we don't yet support bitfields) */ > + if (DECL_C_BIT_FIELD (TREE_OPERAND (x, 1))) > + { > + gcc_assert (gcc::jit::active_playback_ctxt); > + gcc::jit:: > + active_playback_ctxt->add_error (loc, > + "cannot take address of " > + "bit-field"); Can you add a testcase for this error-handling please? [...] > diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c > index a332fe8..0678d07 100644 > --- a/gcc/jit/jit-recording.c > +++ b/gcc/jit/jit-recording.c [...] > +/* Override the default implementation of > + recording::memento::write_to_dump. Dump each bit field > + by dumping a line of the form: > + TYPE NAME:WIDTH; > + so that we can build up a struct/union field-byfield. */ Nit: "field by field", I suppose. [...] > +/* Implementation of recording::memento::write_reproducer for bitfields. */ > + > +void > +recording::bitfield::write_reproducer (reproducer &r) > +{ > + const char *id = r.make_identifier (this, "bitfield"); > + r.write (" gcc_jit_field *%s =\n" > + " gcc_jit_context_new_bitfield (%s,\n" > + " %s, /* gcc_jit_location *loc */\n" > + " %s, /* gcc_jit_type *type, */\n" > + " %d, /* int *width, */\n" This writes a stray "*" in the auto-generated comment here - isn't it an "int", rather than an "int *"? > + " %s); /* const char *name */\n", > + id, > + r.get_identifier (get_context ()), > + r.get_identifier (m_loc), > + r.get_identifier_as_type (m_type), > + m_width, > + m_name->get_debug_string ()); > +} > + > /* The implementation of class gcc::jit::recording::compound_type */ > > /* The constructor for gcc::jit::recording::compound_type. */ > diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h > index b9f2250..5e2d64e 100644 > --- a/gcc/jit/jit-recording.h > +++ b/gcc/jit/jit-recording.h [...] > @@ -822,9 +828,9 @@ public: > compound_type * get_container () const { return m_container; } > void set_container (compound_type *c) { m_container = c; } > > - void replay_into (replayer *) FINAL OVERRIDE; > + void replay_into (replayer *); > > - void write_to_dump (dump &d) FINAL OVERRIDE; > + void write_to_dump (dump &d); Although these are no longer FINAL, aren't they still "OVERRIDE"? > > playback::field * > playback_field () const > @@ -833,16 +839,40 @@ public: > } > > private: > - string * make_debug_string () FINAL OVERRIDE; > - void write_reproducer (reproducer &r) FINAL OVERRIDE; > + string * make_debug_string (); > + void write_reproducer (reproducer &r); Likewise. > -private: > +protected: > location *m_loc; > type *m_type; > string *m_name; > compound_type *m_container; > }; > > + > +class bitfield : public field > +{ > +public: > + bitfield (context *ctxt, > + location *loc, > + type *type, > + int width, > + string *name) > + : field (ctxt, loc, type, name) > + { m_width = width; } Please use: m_width (width) member init syntax, rather than doing it in the body of the ctor. [...] > diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c > index e4f17f8..f1265a5 100644 > --- a/gcc/jit/libgccjit.c > +++ b/gcc/jit/libgccjit.c [...] > @@ -556,6 +560,35 @@ gcc_jit_context_new_field (gcc_jit_context *ctxt, > > /* Public entrypoint. See description in libgccjit.h. > > + After error-checking, the real work is done by the > + gcc::jit::recording::context::new_bitfield method, in > + jit-recording.c. */ > + > +gcc_jit_field * > +gcc_jit_context_new_bitfield (gcc_jit_context *ctxt, > + gcc_jit_location *loc, > + gcc_jit_type *type, > + int width, > + const char *name) > +{ > + RETURN_NULL_IF_FAIL (ctxt, NULL, NULL, "NULL context"); > + JIT_LOG_FUNC (ctxt->get_logger ()); > + /* LOC can be NULL. */ > + RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type"); > + RETURN_NULL_IF_FAIL (width > 0, ctxt, NULL, "invalid width"); Would be better to be more precise, something like: RETURN_NULL_IF_FAIL_PRINTF2 ( width > 0, ctxt, loc, "invalid width %d for bitfield \"%s\" (must be > 0)", name, width); or somesuch, moving the check for NULL name before this. In fact, it would be ideal to move the check for NULL name to before that for the type, and to specify the name in that message too (the more pertinent information we can give the user, the better). > + RETURN_NULL_IF_FAIL (name, ctxt, loc, "NULL name"); > + RETURN_NULL_IF_FAIL_PRINTF2 ( > + type->has_known_size (), > + ctxt, loc, > + "unknown size for field \"%s\" (type: %s)", > + name, > + type->get_debug_string ()); > + > + return (gcc_jit_field *)ctxt->new_bitfield (loc, type, width, name); > +} > + > +/* Public entrypoint. See description in libgccjit.h. > + > After error-checking, this calls the trivial > gcc::jit::recording::memento::as_object method (a field is a > memento), in jit-recording.h. */ [...] Thanks Dave
David Malcolm writes: > Thanks for working on this; sorry for the delay in reviewing it. > > Overall, this looks close to being ready, but I have a few notes: > > [...] Cool thanks for the review. I'll adress your comments. When you have time if you could take a look to the other pending patch I have (this is way smaller) would be great. Bests Andrea
Hi all, second version here of the gcc_jit_context_new_bitfield patch addressing review comments. Checked with make check-jit runs clean. Bests Andrea 2019-06-20 Andrea Corallo andrea.corallo@arm.com * docs/topics/compatibility.rst (LIBGCCJIT_ABI_12): New ABI tag. * docs/topics/types.rst: Add gcc_jit_context_new_bitfield. * jit-common.h (namespace recording): Add class bitfield. * jit-playback.c: (DECL_C_BIT_FIELD, SET_DECL_C_BIT_FIELD): Add macros. (playback::context::new_bitfield): New method. (playback::compound_type::set_fields): Add bitfield support. (playback::lvalue::mark_addressable): Was jit_mark_addressable make this a method of lvalue plus return a bool to communicate success. (playback::lvalue::get_address): Check for jit_mark_addressable return value. * jit-playback.h (new_bitfield): New method. (class bitfield): New class. (class lvalue): Add jit_mark_addressable method. * jit-recording.c (recording::context::new_bitfield): New method. (recording::bitfield::replay_into): New method. (recording::bitfield::write_to_dump): Likewise. (recording::bitfield::make_debug_string): Likewise. (recording::bitfield::write_reproducer): Likewise. * jit-recording.h (class context): Add new_bitfield method. (class field): Make it derivable by class bitfield. (class bitfield): Add new class. * libgccjit++.h (class context): Add new_bitfield method. * libgccjit.c (struct gcc_jit_bitfield): New structure. (gcc_jit_context_new_bitfield): New function. * libgccjit.h (LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield) New macro. (gcc_jit_context_new_bitfield): New function. * libgccjit.map (LIBGCCJIT_ABI_12) New ABI tag. 2019-06-20 Andrea Corallo andrea.corallo@arm.com * jit.dg/all-non-failing-tests.h: Add test-accessing-bitfield.c. * jit.dg/test-accessing-bitfield.c: New testcase. * jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-type.c: Likewise. * jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-width.c: Likewise. * jit.dg/test-error-gcc_jit_lvalue_get_address-bitfield.c: Likewise.
On Mon, 2019-06-24 at 15:26 +0000, Andrea Corallo wrote: > Hi all, > second version here of the gcc_jit_context_new_bitfield patch > addressing > review comments. > > Checked with make check-jit runs clean. > > Bests > > Andrea > > 2019-06-20 Andrea Corallo andrea.corallo@arm.com > > * docs/topics/compatibility.rst (LIBGCCJIT_ABI_12): New ABI tag. > * docs/topics/types.rst: Add gcc_jit_context_new_bitfield. > * jit-common.h (namespace recording): Add class bitfield. > * jit-playback.c: > (DECL_C_BIT_FIELD, SET_DECL_C_BIT_FIELD): Add macros. > (playback::context::new_bitfield): New method. > (playback::compound_type::set_fields): Add bitfield support. > (playback::lvalue::mark_addressable): Was jit_mark_addressable make > this > a method of lvalue plus return a bool to communicate success. > (playback::lvalue::get_address): Check for jit_mark_addressable > return > value. > * jit-playback.h (new_bitfield): New method. > (class bitfield): New class. > (class lvalue): Add jit_mark_addressable method. > * jit-recording.c (recording::context::new_bitfield): New method. > (recording::bitfield::replay_into): New method. > (recording::bitfield::write_to_dump): Likewise. > (recording::bitfield::make_debug_string): Likewise. > (recording::bitfield::write_reproducer): Likewise. > * jit-recording.h (class context): Add new_bitfield method. > (class field): Make it derivable by class bitfield. > (class bitfield): Add new class. > * libgccjit++.h (class context): Add new_bitfield method. > * libgccjit.c (struct gcc_jit_bitfield): New structure. > (gcc_jit_context_new_bitfield): New function. > * libgccjit.h > (LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield) New macro. > (gcc_jit_context_new_bitfield): New function. > * libgccjit.map (LIBGCCJIT_ABI_12) New ABI tag. > > > 2019-06-20 Andrea Corallo andrea.corallo@arm.com > > * jit.dg/all-non-failing-tests.h: Add test-accessing-bitfield.c. > * jit.dg/test-accessing-bitfield.c: New testcase. > * jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-type.c: > Likewise. > * jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-width.c: > Likewise. > * jit.dg/test-error-gcc_jit_lvalue_get_address-bitfield.c: > Likewise. Thanks for the updated patch. [...] > diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c > index b74495c..a6a9e2d 100644 > --- a/gcc/jit/jit-playback.c > +++ b/gcc/jit/jit-playback.c > @@ -47,6 +47,12 @@ along with GCC; see the file COPYING3. If not see > #include "jit-builtins.h" > #include "jit-tempdir.h" > > +/* Compare with gcc/c-family/c-common.h > + This is redifined here to avoid depending from the C frontend. */ > +#define DECL_C_BIT_FIELD(NODE) \ > + (DECL_LANG_FLAG_4 (FIELD_DECL_CHECK (NODE)) == 1) > +#define SET_DECL_C_BIT_FIELD(NODE) \ > + (DECL_LANG_FLAG_4 (FIELD_DECL_CHECK (NODE)) = 1) Can you rename these (and their users) from *_C_BIT_FIELD to *_JIT_BIT_FIELD (and update the comment please). Nit: "redifined" -> "redefined". [...] > diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h > index 9a10418..f7af8e9 100644 > --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h > +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h > @@ -8,6 +8,13 @@ > hooks provided by each test case. */ > #define COMBINED_TEST > > +/* test-accessing-bitfield.c */ > +#define create_code create_code_accessing_bitfield > +#define verify_code verify_code_accessing_bitfield > +#include "test-accessing-bitfield.c" > +#undef create_code > +#undef verify_code > + > /* test-accessing-struct.c */ > #define create_code create_code_accessing_struct > #define verify_code verify_code_accessing_struct You should also add an entry containing create_code_accessing_bitfield verify_code_accessing_bitfield to the "testcases" array at the bottom of the file, so that the new tests are also run as part of test-combination.c and test-threads.c; hopefully there are no conflicts with existing tests in the suite (e.g. names of generated functions within the gcc_jit_context). [...] > diff --git a/gcc/testsuite/jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-width.c b/gcc/testsuite/jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-width.c > new file mode 100644 > index 0000000..6cb151b > --- /dev/null > +++ b/gcc/testsuite/jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-width.c > @@ -0,0 +1,40 @@ > +#include <stdlib.h> > +#include <stdio.h> > + > +#include "libgccjit.h" > + > +#include "harness.h" > + > +/* Try to declare a bit-field with invalid width. */ > + > +void > +create_code (gcc_jit_context *ctxt, void *user_data) > +{ > + gcc_jit_type *short_type = > + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_SHORT); > + gcc_jit_field *i = > + gcc_jit_context_new_bitfield (ctxt, > + NULL, > + short_type, > + 3, > + "i"); > + gcc_jit_field *j = > + gcc_jit_context_new_bitfield (ctxt, > + NULL, > + short_type, > + 57, > + "j"); > + gcc_jit_field *fields[] = {i, j}; > + gcc_jit_context_new_struct_type (ctxt, NULL, "bit_foo", 2, fields); > +} > + > +void > +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) > +{ > + CHECK_VALUE (result, NULL); > + > + /* Verify that the correct error message was emitted. */ > + CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt), > + "width of bit-field j (width: 57) is wider than its type " > + "(width: 16)"); I'm slightly nervous here that the test is hardcoding that a short is 16 bits (AFAIK, we guarantee that a short >= 16 bits; I'm not sure if we guarantee that it's equal to 16 bits). > diff --git a/gcc/testsuite/jit.dg/test-error-gcc_jit_lvalue_get_address-bitfield.c b/gcc/testsuite/jit.dg/test-error-gcc_jit_lvalue_get_address-bitfield.c > new file mode 100644 > index 0000000..35a7c8b > --- /dev/null > +++ b/gcc/testsuite/jit.dg/test-error-gcc_jit_lvalue_get_address-bitfield.c > @@ -0,0 +1,77 @@ > +#include <stdlib.h> > +#include <stdio.h> > + > +#include "libgccjit.h" > + > +#include "harness.h" > + > +/* Try to dereference a bit-field. */ > + > +void > +create_code (gcc_jit_context *ctxt, void *user_data) > +{ > + /* Let's try to inject the equivalent of: > + > + struct bit_foo > + { > + int i:3; > + int j:3; > + }; > + > + int * > + test_access (struct bit_foo *f) > + { > + return &(f->j); > + } > + */ > + gcc_jit_type *int_type = > + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT); > + gcc_jit_field *i = > + gcc_jit_context_new_bitfield (ctxt, > + NULL, > + int_type, > + 3, > + "i"); > + gcc_jit_field *j = > + gcc_jit_context_new_bitfield (ctxt, > + NULL, > + int_type, > + 3, > + "j"); > + gcc_jit_field *fields[] = {i, j}; > + gcc_jit_struct *struct_type = > + gcc_jit_context_new_struct_type (ctxt, NULL, "bit_foo", 2, fields); > + gcc_jit_type *ptr_type = > + gcc_jit_type_get_pointer (gcc_jit_struct_as_type (struct_type)); > + > + /* Build the test function. */ > + gcc_jit_param *param_f = > + gcc_jit_context_new_param (ctxt, NULL, ptr_type, "f"); > + gcc_jit_function *test_fn = > + gcc_jit_context_new_function (ctxt, NULL, > + GCC_JIT_FUNCTION_EXPORTED, > + gcc_jit_type_get_pointer (int_type), > + "test_access", > + 1, ¶m_f, > + 0); > + gcc_jit_block *block = gcc_jit_function_new_block (test_fn, NULL); > + gcc_jit_block_end_with_return ( > + block, > + NULL, > + gcc_jit_lvalue_get_address ( > + gcc_jit_rvalue_dereference_field ( > + gcc_jit_param_as_rvalue (param_f), > + NULL, > + j), > + NULL)); > +} > + > +void > +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) > +{ > + CHECK_VALUE (result, NULL); > + > + /* Verify that the correct error message was emitted. */ > + CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt), > + "cannot take address of bit-field"); > +} As noted in the other review, this testcase could be simplified by just calling gcc_jit_lvalue_get_address directly, without building a function. Dave
Hi David, thanks for the suggestions. Updated version for the bitfield libgccjit support patch here addressing comments. test-error-gcc_jit_context_new_bitfield-invalid-width.c is reworked and now assume that the long of the compiler compiling the test is of the same size of the libgccjit long. I'm not sure this assumption is sufficent, in case is not we have to find another way around this. Checked with make check-jit runs clean. Bests Andrea 2019-06-20 Andrea Corallo andrea.corallo@arm.com * docs/topics/compatibility.rst (LIBGCCJIT_ABI_12): New ABI tag. * docs/topics/types.rst: Add gcc_jit_context_new_bitfield. * jit-common.h (namespace recording): Add class bitfield. * jit-playback.c: (DECL_C_BIT_FIELD, SET_DECL_C_BIT_FIELD): Add macros. (playback::context::new_bitfield): New method. (playback::compound_type::set_fields): Add bitfield support. (playback::lvalue::mark_addressable): Was jit_mark_addressable make this a method of lvalue plus return a bool to communicate success. (playback::lvalue::get_address): Check for jit_mark_addressable return value. * jit-playback.h (new_bitfield): New method. (class bitfield): New class. (class lvalue): Add jit_mark_addressable method. * jit-recording.c (recording::context::new_bitfield): New method. (recording::bitfield::replay_into): New method. (recording::bitfield::write_to_dump): Likewise. (recording::bitfield::make_debug_string): Likewise. (recording::bitfield::write_reproducer): Likewise. * jit-recording.h (class context): Add new_bitfield method. (class field): Make it derivable by class bitfield. (class bitfield): Add new class. * libgccjit++.h (class context): Add new_bitfield method. * libgccjit.c (struct gcc_jit_bitfield): New structure. (gcc_jit_context_new_bitfield): New function. * libgccjit.h (LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield) New macro. (gcc_jit_context_new_bitfield): New function. * libgccjit.map (LIBGCCJIT_ABI_12) New ABI tag. 2019-06-20 Andrea Corallo andrea.corallo@arm.com * jit.dg/all-non-failing-tests.h: Add test-accessing-bitfield.c. * jit.dg/test-accessing-bitfield.c: New testcase. * jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-type.c: Likewise. * jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-width.c: Likewise. * jit.dg/test-error-gcc_jit_lvalue_get_address-bitfield.c: Likewise.
On Wed, 2019-06-26 at 11:07 +0000, Andrea Corallo wrote: > Hi David, > thanks for the suggestions. > Updated version for the bitfield libgccjit support patch here > addressing comments. > > test-error-gcc_jit_context_new_bitfield-invalid-width.c is reworked > and now assume that the long of the compiler compiling the test is of > the same size of the libgccjit long. > I'm not sure this assumption is sufficent, in case is not we have to > find another way around this. > > Checked with make check-jit runs clean. > > Bests > > Andrea > > > 2019-06-20 Andrea Corallo andrea.corallo@arm.com > > * docs/topics/compatibility.rst (LIBGCCJIT_ABI_12): New ABI tag. > * docs/topics/types.rst: Add gcc_jit_context_new_bitfield. > * jit-common.h (namespace recording): Add class bitfield. > * jit-playback.c: > (DECL_C_BIT_FIELD, SET_DECL_C_BIT_FIELD): Add macros. > (playback::context::new_bitfield): New method. > (playback::compound_type::set_fields): Add bitfield support. > (playback::lvalue::mark_addressable): Was jit_mark_addressable make > this > a method of lvalue plus return a bool to communicate success. > (playback::lvalue::get_address): Check for jit_mark_addressable > return > value. > * jit-playback.h (new_bitfield): New method. > (class bitfield): New class. > (class lvalue): Add jit_mark_addressable method. > * jit-recording.c (recording::context::new_bitfield): New method. > (recording::bitfield::replay_into): New method. > (recording::bitfield::write_to_dump): Likewise. > (recording::bitfield::make_debug_string): Likewise. > (recording::bitfield::write_reproducer): Likewise. > * jit-recording.h (class context): Add new_bitfield method. > (class field): Make it derivable by class bitfield. > (class bitfield): Add new class. > * libgccjit++.h (class context): Add new_bitfield method. > * libgccjit.c (struct gcc_jit_bitfield): New structure. > (gcc_jit_context_new_bitfield): New function. > * libgccjit.h > (LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield) New macro. > (gcc_jit_context_new_bitfield): New function. > * libgccjit.map (LIBGCCJIT_ABI_12) New ABI tag. > > > 2019-06-20 Andrea Corallo andrea.corallo@arm.com > > * jit.dg/all-non-failing-tests.h: Add test-accessing-bitfield.c. > * jit.dg/test-accessing-bitfield.c: New testcase. > * jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-type.c: > Likewise. > * jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-width.c: > Likewise. > * jit.dg/test-error-gcc_jit_lvalue_get_address-bitfield.c: > Likewise. Thanks for the updated patch. One last nit: [...] > diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c > index b74495c..ae8a732 100644 > --- a/gcc/jit/jit-playback.c > +++ b/gcc/jit/jit-playback.c > @@ -47,6 +47,12 @@ along with GCC; see the file COPYING3. If not see > #include "jit-builtins.h" > #include "jit-tempdir.h" > > +/* Compare with gcc/c-family/c-common.h We should say what to compare it against: Compare with "DECL_C_BIT_FIELD" etc in gcc/c-family/c-common.h > + This is redefined here to avoid depending from the C frontend. */ s/from/on/ also, FWIW > +#define DECL_JIT_BIT_FIELD(NODE) \ > + (DECL_LANG_FLAG_4 (FIELD_DECL_CHECK (NODE)) == 1) > +#define SET_DECL_JIT_BIT_FIELD(NODE) \ > + (DECL_LANG_FLAG_4 (FIELD_DECL_CHECK (NODE)) = 1) With that change, the patch is good for trunk - thanks for all your work on this. Are you working on getting SVN commit access, or do you want me to commit your two patches on your behalf? Dave
Hi Dave, last version for this patch addressing the suggestion about the JIT_BIT_FIELD macros comment description. Thank you for all the suggestions. Regarding the write access please see my previous answer into the binary op patch thread. Bests Andrea 2019-06-20 Andrea Corallo andrea.corallo@arm.com * docs/topics/compatibility.rst (LIBGCCJIT_ABI_12): New ABI tag. * docs/topics/types.rst: Add gcc_jit_context_new_bitfield. * jit-common.h (namespace recording): Add class bitfield. * jit-playback.c: (DECL_JIT_BIT_FIELD, SET_DECL_JIT_BIT_FIELD): Add macros. (playback::context::new_bitfield): New method. (playback::compound_type::set_fields): Add bitfield support. (playback::lvalue::mark_addressable): Was jit_mark_addressable make this a method of lvalue plus return a bool to communicate success. (playback::lvalue::get_address): Check for jit_mark_addressable return value. * jit-playback.h (new_bitfield): New method. (class bitfield): New class. (class lvalue): Add jit_mark_addressable method. * jit-recording.c (recording::context::new_bitfield): New method. (recording::bitfield::replay_into): New method. (recording::bitfield::write_to_dump): Likewise. (recording::bitfield::make_debug_string): Likewise. (recording::bitfield::write_reproducer): Likewise. * jit-recording.h (class context): Add new_bitfield method. (class field): Make it derivable by class bitfield. (class bitfield): Add new class. * libgccjit++.h (class context): Add new_bitfield method. * libgccjit.c (struct gcc_jit_bitfield): New structure. (gcc_jit_context_new_bitfield): New function. * libgccjit.h (LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield) New macro. (gcc_jit_context_new_bitfield): New function. * libgccjit.map (LIBGCCJIT_ABI_12) New ABI tag. 2019-06-20 Andrea Corallo andrea.corallo@arm.com * jit.dg/all-non-failing-tests.h: Add test-accessing-bitfield.c. * jit.dg/test-accessing-bitfield.c: New testcase. * jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-type.c: Likewise. * jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-width.c: Likewise. * jit.dg/test-error-gcc_jit_lvalue_get_address-bitfield.c: Likewise.
Andrea Corallo writes: > Hi Dave, > last version for this patch addressing the suggestion about the > JIT_BIT_FIELD macros comment description. > > Thank you for all the suggestions. > > Regarding the write access please see my previous answer into the binary > op patch thread. > > Bests > Andrea > > > 2019-06-20 Andrea Corallo andrea.corallo@arm.com > > * docs/topics/compatibility.rst (LIBGCCJIT_ABI_12): New ABI tag. > * docs/topics/types.rst: Add gcc_jit_context_new_bitfield. > * jit-common.h (namespace recording): Add class bitfield. > * jit-playback.c: > (DECL_JIT_BIT_FIELD, SET_DECL_JIT_BIT_FIELD): Add macros. > (playback::context::new_bitfield): New method. > (playback::compound_type::set_fields): Add bitfield support. > (playback::lvalue::mark_addressable): Was jit_mark_addressable make this > a method of lvalue plus return a bool to communicate success. > (playback::lvalue::get_address): Check for jit_mark_addressable return > value. > * jit-playback.h (new_bitfield): New method. > (class bitfield): New class. > (class lvalue): Add jit_mark_addressable method. > * jit-recording.c (recording::context::new_bitfield): New method. > (recording::bitfield::replay_into): New method. > (recording::bitfield::write_to_dump): Likewise. > (recording::bitfield::make_debug_string): Likewise. > (recording::bitfield::write_reproducer): Likewise. > * jit-recording.h (class context): Add new_bitfield method. > (class field): Make it derivable by class bitfield. > (class bitfield): Add new class. > * libgccjit++.h (class context): Add new_bitfield method. > * libgccjit.c (struct gcc_jit_bitfield): New structure. > (gcc_jit_context_new_bitfield): New function. > * libgccjit.h > (LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield) New macro. > (gcc_jit_context_new_bitfield): New function. > * libgccjit.map (LIBGCCJIT_ABI_12) New ABI tag. > > > 2019-06-20 Andrea Corallo andrea.corallo@arm.com > > * jit.dg/all-non-failing-tests.h: Add test-accessing-bitfield.c. > * jit.dg/test-accessing-bitfield.c: New testcase. > * jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-type.c: > Likewise. > * jit.dg/test-error-gcc_jit_context_new_bitfield-invalid-width.c: > Likewise. > * jit.dg/test-error-gcc_jit_lvalue_get_address-bitfield.c: > Likewise. Hi all, committed into trunk as r273086. Bests Andrea
diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst index abefa56..da64920 100644 --- a/gcc/jit/docs/topics/compatibility.rst +++ b/gcc/jit/docs/topics/compatibility.rst @@ -177,3 +177,8 @@ entrypoints: -------------------- ``LIBGCCJIT_ABI_11`` covers the addition of :func:`gcc_jit_context_add_driver_option` + +``LIBGCCJIT_ABI_12`` +-------------------- +``LIBGCCJIT_ABI_12`` covers the addition of +:func:`gcc_jit_context_new_bitfield` diff --git a/gcc/jit/docs/topics/types.rst b/gcc/jit/docs/topics/types.rst index 1d2dcd4..094f9fd 100644 --- a/gcc/jit/docs/topics/types.rst +++ b/gcc/jit/docs/topics/types.rst @@ -247,6 +247,30 @@ You can model C `struct` types by creating :c:type:`gcc_jit_struct *` and underlying string, so it is valid to pass in a pointer to an on-stack buffer. +.. function:: gcc_jit_field *\ + gcc_jit_context_new_bitfield (gcc_jit_context *ctxt,\ + gcc_jit_location *loc,\ + gcc_jit_type *type,\ + int width,\ + const char *name) + + Construct a new bit field, with the given type width and name. + + The parameter ``name`` must be non-NULL. The call takes a copy of the + underlying string, so it is valid to pass in a pointer to an on-stack + buffer. + + The parameter ``type`` must be an integer type. + + The parameter ``width`` must be a positive integer that does not exceed the + size of ``type``. + + This API entrypoint was added in :ref:`LIBGCCJIT_ABI_12`; you can test + for its presence using + .. code-block:: c + + #ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield + .. function:: gcc_jit_object *\ gcc_jit_field_as_object (gcc_jit_field *field) diff --git a/gcc/jit/jit-common.h b/gcc/jit/jit-common.h index 1d96cc3..e747d96 100644 --- a/gcc/jit/jit-common.h +++ b/gcc/jit/jit-common.h @@ -119,6 +119,7 @@ namespace recording { class union_; class vector_type; class field; + class bitfield; class fields; class function; class block; diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c index b74495c..7676e55 100644 --- a/gcc/jit/jit-playback.c +++ b/gcc/jit/jit-playback.c @@ -31,6 +31,7 @@ along with GCC; see the file COPYING3. If not see #include "convert.h" #include "stor-layout.h" #include "print-tree.h" +#include "c-family/c-common.h" #include "gimplify.h" #include "gcc-driver-name.h" #include "attribs.h" @@ -263,6 +264,48 @@ new_field (location *loc, return new field (decl); } +/* Construct a playback::bitfield instance (wrapping a tree). */ + +playback::field * +playback::context:: +new_bitfield (location *loc, + type *type, + int width, + const char *name) +{ + gcc_assert (type); + gcc_assert (name); + gcc_assert (width); + + /* compare with c/c-decl.c:grokfield, grokdeclarator and + check_bitfield_type_and_width. */ + + tree tree_type = type->as_tree (); + if (TREE_CODE (tree_type) != INTEGER_TYPE + && TREE_CODE (tree_type) != BOOLEAN_TYPE) + { + add_error (loc, "bit-field %s has invalid type", name); + return NULL; + } + tree tree_width = build_int_cst (integer_type_node, width); + if (compare_tree_int (tree_width, TYPE_PRECISION (tree_type)) > 0) + { + add_error (loc, "bit-field %s exceeds its type", name); + return NULL; + } + + tree decl = build_decl (UNKNOWN_LOCATION, FIELD_DECL, + get_identifier (name), type->as_tree ()); + DECL_NONADDRESSABLE_P (decl) = true; + DECL_INITIAL (decl) = tree_width; + SET_DECL_C_BIT_FIELD (decl); + + if (loc) + set_tree_location (decl, loc); + + return new field (decl); +} + /* Construct a playback::compound_type instance (wrapping a tree). */ playback::compound_type * @@ -295,8 +338,15 @@ playback::compound_type::set_fields (const auto_vec<playback::field *> *fields) for (unsigned i = 0; i < fields->length (); i++) { field *f = (*fields)[i]; - DECL_CONTEXT (f->as_tree ()) = t; - fieldlist = chainon (f->as_tree (), fieldlist); + tree x = f->as_tree (); + DECL_CONTEXT (x) = t; + if (DECL_C_BIT_FIELD (x)) + { + unsigned HOST_WIDE_INT width = tree_to_uhwi (DECL_INITIAL (x)); + DECL_SIZE (x) = bitsize_int (width); + DECL_BIT_FIELD (x) = 1; + } + fieldlist = chainon (x, fieldlist); } fieldlist = nreverse (fieldlist); TYPE_FIELDS (t) = fieldlist; @@ -1197,20 +1247,30 @@ dereference (location *loc) return new lvalue (get_context (), datum); } -/* Mark EXP saying that we need to be able to take the +/* Mark the lvalue saying that we need to be able to take the address of it; it should not be allocated in a register. - Compare with e.g. c/c-typeck.c: c_mark_addressable. */ + Compare with e.g. c/c-typeck.c: c_mark_addressable really_atomic_lvalue. + Returns true if successful. */ -static void -jit_mark_addressable (tree exp) +bool +playback::lvalue:: +jit_mark_addressable (location *loc) { - tree x = exp; + tree x = as_tree ();; while (1) switch (TREE_CODE (x)) { case COMPONENT_REF: - /* (we don't yet support bitfields) */ + if (DECL_C_BIT_FIELD (TREE_OPERAND (x, 1))) + { + gcc_assert (gcc::jit::active_playback_ctxt); + gcc::jit:: + active_playback_ctxt->add_error (loc, + "cannot take address of " + "bit-field"); + return false; + } /* fallthrough */ case ADDR_EXPR: case ARRAY_REF: @@ -1222,7 +1282,7 @@ jit_mark_addressable (tree exp) case COMPOUND_LITERAL_EXPR: case CONSTRUCTOR: TREE_ADDRESSABLE (x) = 1; - return; + return true; case VAR_DECL: case CONST_DECL: @@ -1234,7 +1294,7 @@ jit_mark_addressable (tree exp) TREE_ADDRESSABLE (x) = 1; /* fallthrough */ default: - return; + return true; } } @@ -1251,8 +1311,10 @@ get_address (location *loc) tree ptr = build1 (ADDR_EXPR, t_ptrtype, t_lvalue); if (loc) get_context ()->set_tree_location (ptr, loc); - jit_mark_addressable (t_lvalue); - return new rvalue (get_context (), ptr); + if (jit_mark_addressable (loc)) + return new rvalue (get_context (), ptr); + else + return NULL; } /* The wrapper subclasses are GC-managed, but can own non-GC memory. diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h index bc4de9c..c59f614 100644 --- a/gcc/jit/jit-playback.h +++ b/gcc/jit/jit-playback.h @@ -75,6 +75,12 @@ public: type *type, const char *name); + field * + new_bitfield (location *loc, + type *type, + int width, + const char *name); + compound_type * new_compound_type (location *loc, const char *name, @@ -426,6 +432,8 @@ private: tree m_inner; }; +class bitfield : public field {}; + class function : public wrapper { public: @@ -614,6 +622,8 @@ public: rvalue * get_address (location *loc); +private: + bool jit_mark_addressable (location *loc); }; class param : public lvalue @@ -703,4 +713,3 @@ extern playback::context *active_playback_ctxt; } // namespace gcc #endif /* JIT_PLAYBACK_H */ - diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c index a332fe8..0678d07 100644 --- a/gcc/jit/jit-recording.c +++ b/gcc/jit/jit-recording.c @@ -872,6 +872,24 @@ recording::context::new_field (recording::location *loc, return result; } +/* Create a recording::bitfield instance and add it to this context's list + of mementos. + + Implements the post-error-checking part of + gcc_jit_context_new_bitfield. */ + +recording::field * +recording::context::new_bitfield (recording::location *loc, + recording::type *type, + int width, + const char *name) +{ + recording::field *result = + new recording::bitfield (this, loc, type, width, new_string (name)); + record (result); + return result; +} + /* Create a recording::struct_ instance and add it to this context's list of mementos and list of compound types. @@ -2999,6 +3017,66 @@ recording::field::write_reproducer (reproducer &r) m_name->get_debug_string ()); } +/* The implementation of class gcc::jit::recording::bitfield. */ + +/* Implementation of pure virtual hook recording::memento::replay_into + for recording::bitfield. */ + +void +recording::bitfield::replay_into (replayer *r) +{ + set_playback_obj (r->new_bitfield (playback_location (r, m_loc), + m_type->playback_type (), + m_width, + playback_string (m_name))); +} + +/* Override the default implementation of + recording::memento::write_to_dump. Dump each bit field + by dumping a line of the form: + TYPE NAME:WIDTH; + so that we can build up a struct/union field-byfield. */ + +void +recording::bitfield::write_to_dump (dump &d) +{ + d.write (" %s %s:%d;\n", + m_type->get_debug_string (), + m_name->c_str (), + m_width); +} + +/* Implementation of recording::memento::make_debug_string for + results of new_bitfield. */ + +recording::string * +recording::bitfield::make_debug_string () +{ + return string::from_printf (m_ctxt, + "%s:%d", + m_name->c_str (), m_width); +} + +/* Implementation of recording::memento::write_reproducer for bitfields. */ + +void +recording::bitfield::write_reproducer (reproducer &r) +{ + const char *id = r.make_identifier (this, "bitfield"); + r.write (" gcc_jit_field *%s =\n" + " gcc_jit_context_new_bitfield (%s,\n" + " %s, /* gcc_jit_location *loc */\n" + " %s, /* gcc_jit_type *type, */\n" + " %d, /* int *width, */\n" + " %s); /* const char *name */\n", + id, + r.get_identifier (get_context ()), + r.get_identifier (m_loc), + r.get_identifier_as_type (m_type), + m_width, + m_name->get_debug_string ()); +} + /* The implementation of class gcc::jit::recording::compound_type */ /* The constructor for gcc::jit::recording::compound_type. */ diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h index b9f2250..5e2d64e 100644 --- a/gcc/jit/jit-recording.h +++ b/gcc/jit/jit-recording.h @@ -95,6 +95,12 @@ public: type *type, const char *name); + field * + new_bitfield (location *loc, + type *type, + int width, + const char *name); + struct_ * new_struct_type (location *loc, const char *name); @@ -822,9 +828,9 @@ public: compound_type * get_container () const { return m_container; } void set_container (compound_type *c) { m_container = c; } - void replay_into (replayer *) FINAL OVERRIDE; + void replay_into (replayer *); - void write_to_dump (dump &d) FINAL OVERRIDE; + void write_to_dump (dump &d); playback::field * playback_field () const @@ -833,16 +839,40 @@ public: } private: - string * make_debug_string () FINAL OVERRIDE; - void write_reproducer (reproducer &r) FINAL OVERRIDE; + string * make_debug_string (); + void write_reproducer (reproducer &r); -private: +protected: location *m_loc; type *m_type; string *m_name; compound_type *m_container; }; + +class bitfield : public field +{ +public: + bitfield (context *ctxt, + location *loc, + type *type, + int width, + string *name) + : field (ctxt, loc, type, name) + { m_width = width; } + + void replay_into (replayer *) FINAL OVERRIDE; + + void write_to_dump (dump &d) FINAL OVERRIDE; + +private: + string * make_debug_string () FINAL OVERRIDE; + void write_reproducer (reproducer &r) FINAL OVERRIDE; + +private: + int m_width; +}; + /* Base class for struct_ and union_ */ class compound_type : public type { diff --git a/gcc/jit/libgccjit++.h b/gcc/jit/libgccjit++.h index 55aebca..e5a769a 100644 --- a/gcc/jit/libgccjit++.h +++ b/gcc/jit/libgccjit++.h @@ -152,6 +152,9 @@ namespace gccjit field new_field (type type_, const std::string &name, location loc = location ()); + field new_bitfield (type type_, int width, const std::string &name, + location loc = location ()); + struct_ new_struct_type (const std::string &name, std::vector<field> &fields, location loc = location ()); @@ -757,6 +760,17 @@ context::new_field (type type_, const std::string &name, location loc) name.c_str ())); } +inline field +context::new_bitfield (type type_, int width, const std::string &name, + location loc) +{ + return field (gcc_jit_context_new_bitfield (m_inner_ctxt, + loc.get_inner_location (), + type_.get_inner_type (), + width, + name.c_str ())); +} + inline struct_ context::new_struct_type (const std::string &name, std::vector<field> &fields, diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c index e4f17f8..f1265a5 100644 --- a/gcc/jit/libgccjit.c +++ b/gcc/jit/libgccjit.c @@ -62,6 +62,10 @@ struct gcc_jit_field : public gcc::jit::recording::field { }; +struct gcc_jit_bitfield : public gcc::jit::recording::bitfield +{ +}; + struct gcc_jit_function : public gcc::jit::recording::function { }; @@ -556,6 +560,35 @@ gcc_jit_context_new_field (gcc_jit_context *ctxt, /* Public entrypoint. See description in libgccjit.h. + After error-checking, the real work is done by the + gcc::jit::recording::context::new_bitfield method, in + jit-recording.c. */ + +gcc_jit_field * +gcc_jit_context_new_bitfield (gcc_jit_context *ctxt, + gcc_jit_location *loc, + gcc_jit_type *type, + int width, + const char *name) +{ + RETURN_NULL_IF_FAIL (ctxt, NULL, NULL, "NULL context"); + JIT_LOG_FUNC (ctxt->get_logger ()); + /* LOC can be NULL. */ + RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type"); + RETURN_NULL_IF_FAIL (width > 0, ctxt, NULL, "invalid width"); + RETURN_NULL_IF_FAIL (name, ctxt, loc, "NULL name"); + RETURN_NULL_IF_FAIL_PRINTF2 ( + type->has_known_size (), + ctxt, loc, + "unknown size for field \"%s\" (type: %s)", + name, + type->get_debug_string ()); + + return (gcc_jit_field *)ctxt->new_bitfield (loc, type, width, name); +} + +/* Public entrypoint. See description in libgccjit.h. + After error-checking, this calls the trivial gcc::jit::recording::memento::as_object method (a field is a memento), in jit-recording.h. */ diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h index beeb747..9c5f23b 100644 --- a/gcc/jit/libgccjit.h +++ b/gcc/jit/libgccjit.h @@ -602,6 +602,21 @@ gcc_jit_context_new_field (gcc_jit_context *ctxt, gcc_jit_type *type, const char *name); +#define LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield + +/* Create a bit field, for use within a struct or union. + + This API entrypoint was added in LIBGCCJIT_ABI_12; you can test for its + presence using + #ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_bitfield +*/ +extern gcc_jit_field * +gcc_jit_context_new_bitfield (gcc_jit_context *ctxt, + gcc_jit_location *loc, + gcc_jit_type *type, + int width, + const char *name); + /* Upcasting from field to object. */ extern gcc_jit_object * gcc_jit_field_as_object (gcc_jit_field *field); diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map index 16f5253..40e1c78 100644 --- a/gcc/jit/libgccjit.map +++ b/gcc/jit/libgccjit.map @@ -174,4 +174,9 @@ LIBGCCJIT_ABI_10 { LIBGCCJIT_ABI_11 { global: gcc_jit_context_add_driver_option; -} LIBGCCJIT_ABI_10; \ No newline at end of file +} LIBGCCJIT_ABI_10; + +LIBGCCJIT_ABI_12 { + global: + gcc_jit_context_new_bitfield; +} LIBGCCJIT_ABI_11; \ No newline at end of file diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h index 9a10418..f7af8e9 100644 --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h @@ -8,6 +8,13 @@ hooks provided by each test case. */ #define COMBINED_TEST +/* test-accessing-bitfield.c */ +#define create_code create_code_accessing_bitfield +#define verify_code verify_code_accessing_bitfield +#include "test-accessing-bitfield.c" +#undef create_code +#undef verify_code + /* test-accessing-struct.c */ #define create_code create_code_accessing_struct #define verify_code verify_code_accessing_struct diff --git a/gcc/testsuite/jit.dg/test-accessing-bitfield.c b/gcc/testsuite/jit.dg/test-accessing-bitfield.c new file mode 100644 index 0000000..6ca8b24 --- /dev/null +++ b/gcc/testsuite/jit.dg/test-accessing-bitfield.c @@ -0,0 +1,130 @@ +#include <stdlib.h> +#include <stdio.h> + +#include "libgccjit.h" + +#include "harness.h" + +struct bit_foo +{ + int i:3; + int x:5; + int y:5; + int z:10; + int j:3; +}; + +void +create_code (gcc_jit_context *ctxt, void *user_data) +{ + /* Let's try to inject the equivalent of: + void + test_access (struct bit_foo *f) + { + f->z = f->x + f->y; + } + */ + gcc_jit_type *void_type = + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID); + gcc_jit_type *int_type = + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT); + gcc_jit_field *i = + gcc_jit_context_new_bitfield (ctxt, + NULL, + int_type, + 3, + "i"); + gcc_jit_field *x = + gcc_jit_context_new_bitfield (ctxt, + NULL, + int_type, + 5, + "x"); + gcc_jit_field *y = + gcc_jit_context_new_bitfield (ctxt, + NULL, + int_type, + 5, + "y"); + gcc_jit_field *z = + gcc_jit_context_new_bitfield (ctxt, + NULL, + int_type, + 10, + "z"); + gcc_jit_field *j = + gcc_jit_context_new_bitfield (ctxt, + NULL, + int_type, + 3, + "j"); + gcc_jit_field *fields[] = {i, x, y, z, j}; + gcc_jit_struct *struct_type = + gcc_jit_context_new_struct_type (ctxt, NULL, "bit_foo", 5, fields); + gcc_jit_type *ptr_type = + gcc_jit_type_get_pointer (gcc_jit_struct_as_type (struct_type)); + + /* Build the test function. */ + gcc_jit_param *param_f = + gcc_jit_context_new_param (ctxt, NULL, ptr_type, "f"); + gcc_jit_function *test_fn = + gcc_jit_context_new_function (ctxt, NULL, + GCC_JIT_FUNCTION_EXPORTED, + void_type, + "test_access", + 1, ¶m_f, + 0); + + /* f->x + f->y */ + gcc_jit_rvalue *sum = + gcc_jit_context_new_binary_op ( + ctxt, NULL, + GCC_JIT_BINARY_OP_PLUS, + int_type, + gcc_jit_lvalue_as_rvalue ( + gcc_jit_rvalue_dereference_field ( + gcc_jit_param_as_rvalue (param_f), + NULL, + x)), + gcc_jit_lvalue_as_rvalue ( + gcc_jit_rvalue_dereference_field ( + gcc_jit_param_as_rvalue (param_f), + NULL, + y))); + + /* f->z = ... */ + gcc_jit_block *block = gcc_jit_function_new_block (test_fn, NULL); + gcc_jit_block_add_assignment ( + block, + NULL, + gcc_jit_rvalue_dereference_field ( + gcc_jit_param_as_rvalue (param_f), + NULL, + z), + sum); + gcc_jit_block_end_with_void_return (block, NULL); +} + +void +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) +{ + typedef void (*fn_type) (struct bit_foo *); + CHECK_NON_NULL (result); + + fn_type test_access = + (fn_type)gcc_jit_result_get_code (result, "test_access"); + CHECK_NON_NULL (test_access); + + struct bit_foo tmp; + tmp.i = 3; + tmp.x = 5; + tmp.y = 7; + tmp.z = 0; + tmp.j = 3; + + /* Call the JIT-generated function. */ + test_access (&tmp); + + /* Verify that the code correctly modified the field "z". */ + CHECK_VALUE (tmp.z, 12); +}