[jit] Fix how locals are created; add BIND_EXPR

David Malcolm dmalcolm@redhat.com
Thu Jan 23 21:32:00 GMT 2014


Committed to dmalcolm/jit:

Attempting to add test coverage uncovered that locals weren't being
associated with their functions, leading to crashes later on in the
compile for various valid constructs.

To fix this, we now set the DECL_CONTEXT of locals to be the
FUNCTION_DECL.  To do so, we also need to put them within a BIND_EXPR
otherwise gimplify_var_or_parm_decl dies, with:
       !DECL_SEEN_IN_BIND_EXPR_P (decl)

Hence we now create a BIND_EXPR for every function with a body, moving
the statement list within it.

gcc/jit/
	* internal-api.h (gcc::jit::function): Add field
	"m_inner_bind_expr".
	* internal-api.c (gcc::jit::function::function): Create a BIND_EXPR
	for all non-imported functions, and put the statement list within
	it.
	(gcc::jit::function::gt_ggc_mx): Visit m_inner_bind_expr.
	(gcc::jit::function::new_local): Set the DECL_CONTEXT of the new
	local to be the function's BIND_EXPR, and prepend the new local
	to said BIND_EXPR's BIND_EXPR_VARS chain.
	(gcc::jit::function::postprocess): Set the DECL_SAVED_TREE of the
	FUNCTION_DECL to be the BIND_EXPR, rather than the statement list.
	The latter is now contained within the former.

gcc/testsuite/
	* jit.dg/test-reading-struct.c: New test, to provide test coverage
	of gcc_jit_type_get_const and gcc_jit_lvalue_access_field, in the
	process uncovering bugs in how locals were handled.
	* jit.dg/test-combination.c: Add usage of test-reading-struct.c.
---
 gcc/jit/ChangeLog.jit                      |  15 ++++
 gcc/jit/internal-api.c                     |  13 ++-
 gcc/jit/internal-api.h                     |   1 +
 gcc/testsuite/ChangeLog.jit                |   7 ++
 gcc/testsuite/jit.dg/test-combination.c    |   7 ++
 gcc/testsuite/jit.dg/test-reading-struct.c | 136 +++++++++++++++++++++++++++++
 6 files changed, 178 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/jit.dg/test-reading-struct.c

diff --git a/gcc/jit/ChangeLog.jit b/gcc/jit/ChangeLog.jit
index 2018094..41f52f2 100644
--- a/gcc/jit/ChangeLog.jit
+++ b/gcc/jit/ChangeLog.jit
@@ -1,5 +1,20 @@
 2014-01-23  David Malcolm  <dmalcolm@redhat.com>
 
+	* internal-api.h (gcc::jit::function): Add field
+	"m_inner_bind_expr".
+	* internal-api.c (gcc::jit::function::function): Create a BIND_EXPR
+	for all non-imported functions, and put the statement list within
+	it.
+	(gcc::jit::function::gt_ggc_mx): Visit m_inner_bind_expr.
+	(gcc::jit::function::new_local): Set the DECL_CONTEXT of the new
+	local to be the function's BIND_EXPR, and prepend the new local
+	to said BIND_EXPR's BIND_EXPR_VARS chain.
+	(gcc::jit::function::postprocess): Set the DECL_SAVED_TREE of the
+	FUNCTION_DECL to be the BIND_EXPR, rather than the statement list.
+	The latter is now contained within the former.
+
+2014-01-23  David Malcolm  <dmalcolm@redhat.com>
+
 	* internal-api.h (gcc::jit::function::add_stmt): New.
 
 	* internal-api.c (gcc::jit::function::add_eval): Replace use of
diff --git a/gcc/jit/internal-api.c b/gcc/jit/internal-api.c
index f7a2618..d0d49cc 100644
--- a/gcc/jit/internal-api.c
+++ b/gcc/jit/internal-api.c
@@ -782,12 +782,16 @@ gcc::jit::function::
 function(gcc::jit::context *ctxt, tree fndecl, enum gcc_jit_function_kind kind)
 : m_ctxt(ctxt),
   m_inner_fndecl (fndecl),
+  m_inner_bind_expr (NULL),
   m_kind (kind)
 {
   if (m_kind != GCC_JIT_FUNCTION_IMPORTED)
     {
+      /* Create a BIND_EXPR, and within it, a statement list.  */
       m_stmt_list = alloc_stmt_list ();
       m_stmt_iter = tsi_start (m_stmt_list);
+      m_inner_bind_expr =
+	build3 (BIND_EXPR, void_type_node, NULL, m_stmt_list, NULL);
     }
   else
     {
@@ -800,6 +804,7 @@ gcc::jit::function::
 gt_ggc_mx ()
 {
   gt_ggc_m_9tree_node (m_inner_fndecl);
+  gt_ggc_m_9tree_node (m_inner_bind_expr);
   gt_ggc_m_9tree_node (m_stmt_list);
 }
 
@@ -821,6 +826,12 @@ new_local (location *loc,
   tree inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
 			   get_identifier (name),
 			   type->as_tree ());
+  DECL_CONTEXT (inner) = this->m_inner_fndecl;
+
+  /* Prepend to BIND_EXPR_VARS: */
+  DECL_CHAIN (inner) = BIND_EXPR_VARS (m_inner_bind_expr);
+  BIND_EXPR_VARS (m_inner_bind_expr) = inner;
+
   if (loc)
     set_tree_location (inner, loc);
   return new lvalue (m_ctxt, inner);
@@ -855,7 +866,7 @@ postprocess ()
 
       /* how to add to function? the following appears to be how to
 	 set the body of a m_inner_fndecl: */
-      DECL_SAVED_TREE(m_inner_fndecl) = m_stmt_list;
+      DECL_SAVED_TREE(m_inner_fndecl) = m_inner_bind_expr;
       //debug_tree (m_inner_fndecl);
 
       /* Convert to gimple: */
diff --git a/gcc/jit/internal-api.h b/gcc/jit/internal-api.h
index 7a5c92a..be1216a 100644
--- a/gcc/jit/internal-api.h
+++ b/gcc/jit/internal-api.h
@@ -367,6 +367,7 @@ private:
 
 private:
   tree m_inner_fndecl;
+  tree m_inner_bind_expr;
   enum gcc_jit_function_kind m_kind;
   tree m_stmt_list;
   tree_stmt_iterator m_stmt_iter;
diff --git a/gcc/testsuite/ChangeLog.jit b/gcc/testsuite/ChangeLog.jit
index 34a558c..5cd4a63 100644
--- a/gcc/testsuite/ChangeLog.jit
+++ b/gcc/testsuite/ChangeLog.jit
@@ -1,3 +1,10 @@
+2014-01-23  David Malcolm  <dmalcolm@redhat.com>
+
+	* jit.dg/test-reading-struct.c: New test, to provide test coverage
+	of gcc_jit_type_get_const and gcc_jit_lvalue_access_field, in the
+	process uncovering bugs in how locals were handled.
+	* jit.dg/test-combination.c: Add usage of test-reading-struct.c.
+
 2014-01-21  David Malcolm  <dmalcolm@redhat.com>
 
 	* jit.dg/test-hello-world.c (code_making_callback): Add usage of
diff --git a/gcc/testsuite/jit.dg/test-combination.c b/gcc/testsuite/jit.dg/test-combination.c
index 00fbee6..6a2372f 100644
--- a/gcc/testsuite/jit.dg/test-combination.c
+++ b/gcc/testsuite/jit.dg/test-combination.c
@@ -59,6 +59,13 @@
 #undef code_making_callback
 #undef verify_code
 
+/* test-reading-struct.c */
+#define code_making_callback code_making_callback_reading_struct
+#define verify_code verify_code_reading_struct
+#include "test-reading-struct.c"
+#undef code_making_callback
+#undef verify_code
+
 /* test-string-literal.c */
 #define code_making_callback code_making_callback_string_literal
 #define verify_code verify_code_string_literal
diff --git a/gcc/testsuite/jit.dg/test-reading-struct.c b/gcc/testsuite/jit.dg/test-reading-struct.c
new file mode 100644
index 0000000..94c36fe
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-reading-struct.c
@@ -0,0 +1,136 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+struct bar
+{
+  int x;
+  int y;
+};
+
+int
+code_making_callback (gcc_jit_context *ctxt, void *user_data)
+{
+  /* Let's try to inject the equivalent of:
+
+     int
+     test_reading (const struct bar *f)
+     {
+       return f->x * f->y;
+     }
+
+     int
+     test_writing ()
+     {
+       struct bar tmp;
+       tmp.x = 5;
+       tmp.y = 7;
+       return test_reading (&tmp);
+     }
+  */
+  gcc_jit_type *int_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+  gcc_jit_field *x =
+    gcc_jit_context_new_field (ctxt,
+                               NULL,
+                               int_type,
+                               "x");
+  gcc_jit_field *y =
+    gcc_jit_context_new_field (ctxt,
+                               NULL,
+                               int_type,
+                               "y");
+  gcc_jit_field *fields[] = {x, y};
+  gcc_jit_type *struct_type =
+    gcc_jit_context_new_struct_type (ctxt, NULL, "bar", 2, fields);
+  gcc_jit_type *const_struct_type = gcc_jit_type_get_const (struct_type);
+  gcc_jit_type *ptr_type = gcc_jit_type_get_pointer (const_struct_type);
+
+  /* Build "test_reading".  */
+  gcc_jit_param *param_f =
+    gcc_jit_context_new_param (ctxt, NULL, ptr_type, "f");
+  gcc_jit_function *fn_test_reading =
+    gcc_jit_context_new_function (ctxt, NULL,
+                                  GCC_JIT_FUNCTION_EXPORTED,
+                                  int_type,
+                                  "test_reading",
+                                  1, &param_f,
+                                  0);
+
+  /* return f->x * f->y; */
+  gcc_jit_function_add_return (
+    fn_test_reading,
+    NULL,
+    gcc_jit_context_new_binary_op (
+      ctxt, NULL,
+      GCC_JIT_BINARY_OP_MULT,
+      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"))));
+
+  /* Build "test_writing".  */
+  gcc_jit_function *fn_test_writing =
+    gcc_jit_context_new_function (ctxt, NULL,
+                                  GCC_JIT_FUNCTION_EXPORTED,
+                                  int_type,
+                                  "test_writing",
+                                  0, NULL,
+                                  0);
+
+  /* struct bar tmp; */
+  gcc_jit_lvalue *local_tmp =
+    gcc_jit_function_new_local (fn_test_writing, NULL,
+				struct_type,
+				"tmp");
+#if 1
+  /* tmp.x = 5; */
+  gcc_jit_function_add_assignment (
+    fn_test_writing, NULL,
+    gcc_jit_lvalue_access_field (local_tmp, NULL, "x"),
+    gcc_jit_context_new_rvalue_from_int (ctxt, int_type, 5));
+
+  /* tmp.y = 7; */
+  gcc_jit_function_add_assignment (
+    fn_test_writing, NULL,
+    gcc_jit_lvalue_access_field (local_tmp, NULL, "y"),
+    gcc_jit_context_new_rvalue_from_int (ctxt, int_type, 7));
+#endif
+
+  /* return test_reading (&tmp); */
+  gcc_jit_rvalue *arg = gcc_jit_lvalue_get_address (local_tmp, NULL);
+  gcc_jit_function_add_return (
+    fn_test_writing,
+    NULL,
+    gcc_jit_context_new_call (
+      ctxt, NULL,
+      fn_test_reading,
+      1, &arg));
+
+  return 0;
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  typedef int (*fn_type) (void);
+  CHECK_NON_NULL (result);
+
+  fn_type test_writing =
+    (fn_type)gcc_jit_result_get_code (result, "test_writing");
+  CHECK_NON_NULL (test_writing);
+
+  /* Verify that the code correctly returns the product of the fields.  */
+  CHECK_VALUE (test_writing (), 35);
+}
+
-- 
1.7.11.7



More information about the Gcc-patches mailing list