Re: The output of gcc_jit_context_dump_to_file() is missing parenthesis in expressions involving casts

Re: The output of gcc_jit_context_dump_to_file() is missing parenthesis in expressions involving casts

[David Malcolm]

On Tue, 2015-06-16 at 14:15 -0400, David Malcolm wrote:
> On Sun, 2015-06-14 at 20:42 -0400, David Malcolm wrote:
> > On Sun, 2015-06-14 at 20:11 +0100, Dibyendu Majumdar wrote:
> > > It seems that in some cases the output is not showing parenthesis
> > > where it should.
> > 
> > Yes, this is a bug.
> > 
> > The dumping fns are somewhat hacky; they have no concept of C precedence
> > order, and are probably misleading on some expressions.  Sorry.
> > 
> > I've filed it as:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66539
> 
> (snip)
> 
> This should be fixed now for gcc trunk (for GCC 6) as of r224531;
> gcc_jit_get_debug_string should now add parentheses where needed,
> mimicking C's precedence order. 

FWIW, in particular, my testcase for the debug string of a chain of
derefs with a cast inside them emits this:

  "((struct node *)ptr->next)->next"

(see gcc/testsuite/jit.dg/test-debug-strings.c).

> If after updating to r224531 you still see any expressions where it gets
> it wrong, please let me know.
> 
> Dave
> 

[PATCH, committed] PR jit/66539: Add parentheses as needed to gcc_jit_object_get_debug_string

[David Malcolm]

gcc_jit_object_get_debug_string wasn't adding parentheses, leading to
misleading dumps for certain compound expressions.

This patch adds:
  rvalue::get_debug_string_parens
  rvalue::get_precedence
methods, mimicing C's precedence rules, using them in the get_debug_string
implementation so that we get parentheses where needed when generating
debug strings for compound expressions.

Tested via "make check-jit"; takes the total "expected passes" of
jit.sum from 7609 to 7699.

Committed to trunk as r224531.

gcc/jit/ChangeLog:
	PR jit/66539
	* jit-recording.c: Within namespace gcc::jit::recording::
	(rvalue::get_debug_string_parens): New function.
	(binary_op::make_debug_string): Update to mimic C precedence
	rules.
	(binary_op_precedence): New array.
	(binary_op::get_precedence): New function.
	(comparison::make_debug_string): Update to mimic C precedence
	rules.
	(comparison_precedence): New array.
	(comparison::get_precedence): New function.
	(cast::make_debug_string): Update to mimic C precedence rules.
	(call::make_debug_string): Likewise.
	(call_through_ptr::make_debug_string): Likewise.
	(array_access::make_debug_string): Likewise.
	(access_field_of_lvalue::make_debug_string): Likewise.
	(access_field_rvalue::make_debug_string): Likewise.
	(dereference_field_rvalue::make_debug_string): Likewise.
	(dereference_rvalue::make_debug_string): Likewise.
	(get_address_of_lvalue::make_debug_string): Likewise.
	* jit-recording.h: Within namespace gcc::jit::recording::
	(precedence): New enum.
	(rvalue::rvalue): Initialize field "m_parenthesized_string".
	(rvalue::get_debug_string_parens): New method.
	(rvalue::get_precedence): New pure virtual function.
	(rvalue::m_parenthesized_string): New field.
	(param::get_precedence): New function.
	(global::get_precedence): New function.
	(memento_of_new_rvalue_from_const::get_precedence): New function.
	(memento_of_new_string_literal::get_precedence): New function.
	(unary_op::get_precedence): New function.
	(binary_op::get_precedence): New function.
	(comparison::get_precedence): New function.
	(cast::get_precedence): New function.
	(call::get_precedence): New function.
	(call_through_ptr::get_precedence): New function.
	(array_access::get_precedence): New function.
	(access_field_of_lvalue::get_precedence): New function.
	(access_field_rvalue::get_precedence): New function.
	(dereference_field_rvalue::get_precedence): New function.
	(dereference_rvalue::get_precedence): New function.
	(get_address_of_lvalue::get_precedence): New function.
	(local::get_precedence): New function.

gcc/testsuite/ChangeLog:
	PR jit/66539
	* jit.dg/all-non-failing-tests.h: Add test-debug-strings.c.
	* jit.dg/test-debug-strings.c: New test case.
	* jit.dg/test-quadratic.c (make_calc_discriminant): Verify that
	the discriminant has a sane debug string.
---
 gcc/jit/jit-recording.c                      | 150 ++++++++++++++++++---
 gcc/jit/jit-recording.h                      |  49 ++++++-
 gcc/testsuite/jit.dg/all-non-failing-tests.h |   7 +
 gcc/testsuite/jit.dg/test-debug-strings.c    | 190 +++++++++++++++++++++++++++
 gcc/testsuite/jit.dg/test-quadratic.c        |  28 ++--
 5 files changed, 393 insertions(+), 31 deletions(-)
 create mode 100644 gcc/testsuite/jit.dg/test-debug-strings.c

diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index 32d7f31..f379b58 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -3021,6 +3021,63 @@ recording::rvalue::access_as_rvalue (reproducer &r)
   return r.get_identifier (this);
 }
 
+/* Return a debug string for the given rvalue, wrapping it in parentheses
+   if needed to mimic C's precedence rules, i.e. if OUTER_PREC is of
+   stronger precedence that this rvalue's precedence.
+
+   For example, given:
+
+           MULT
+          /    \
+       PLUS     MINUS
+      /    \   /     \
+     A      B C       D
+
+   we want to emit:
+
+     (A + B) * (C - D)
+
+   since MULT has strong precedence than PLUS and MINUS, whereas for:
+
+           PLUS
+          /    \
+       MULT     DIVIDE
+      /    \   /      \
+     A      B C        D
+
+   we can simply emit:
+
+     A * B + C / D
+
+   since PLUS has weaker precedence than MULT and DIVIDE.  */
+
+const char *
+recording::rvalue::get_debug_string_parens (enum precedence outer_prec)
+{
+  enum precedence this_prec = get_precedence ();
+
+  /* If this_prec has stronger precedence than outer_prec, we don't
+     need to wrap this in parens within the outer debug string.
+     Stronger precedences occur earlier than weaker within the enum,
+     so this is a less than test.  Equal precedences don't need
+     parentheses.  */
+  if (this_prec <= outer_prec)
+    return get_debug_string();
+
+  /* Otherwise, we need parentheses.  */
+
+  /* Lazily-build and cache m_parenthesized_string.  */
+  if (!m_parenthesized_string)
+    {
+      const char *debug_string = get_debug_string ();
+      m_parenthesized_string = string::from_printf (get_context (),
+						    "(%s)",
+						    debug_string);
+    }
+  gcc_assert (m_parenthesized_string);
+  return m_parenthesized_string->c_str ();
+}
+
 
 /* The implementation of class gcc::jit::recording::lvalue.  */
 
@@ -4251,11 +4308,12 @@ static const char * const binary_op_strings[] = {
 recording::string *
 recording::binary_op::make_debug_string ()
 {
+  enum precedence prec = get_precedence ();
   return string::from_printf (m_ctxt,
 			      "%s %s %s",
-			      m_a->get_debug_string (),
+			      m_a->get_debug_string_parens (prec),
 			      binary_op_strings[m_op],
-			      m_b->get_debug_string ());
+			      m_b->get_debug_string_parens (prec));
 }
 
 static const char * const binary_op_reproducer_strings[] = {
@@ -4295,6 +4353,31 @@ recording::binary_op::write_reproducer (reproducer &r)
 	   r.get_identifier_as_rvalue (m_b));
 }
 
+namespace recording {
+static const enum precedence binary_op_precedence[] = {
+  PRECEDENCE_ADDITIVE, /* GCC_JIT_BINARY_OP_PLUS */
+  PRECEDENCE_ADDITIVE, /* GCC_JIT_BINARY_OP_MINUS */
+
+  PRECEDENCE_MULTIPLICATIVE, /* GCC_JIT_BINARY_OP_MULT */
+  PRECEDENCE_MULTIPLICATIVE, /* GCC_JIT_BINARY_OP_DIVIDE */
+  PRECEDENCE_MULTIPLICATIVE, /* GCC_JIT_BINARY_OP_MODULO */
+
+  PRECEDENCE_BITWISE_AND, /* GCC_JIT_BINARY_OP_BITWISE_AND */
+  PRECEDENCE_BITWISE_XOR, /* GCC_JIT_BINARY_OP_BITWISE_XOR */
+  PRECEDENCE_BITWISE_IOR, /* GCC_JIT_BINARY_OP_BITWISE_OR */
+  PRECEDENCE_LOGICAL_AND, /* GCC_JIT_BINARY_OP_LOGICAL_AND */
+  PRECEDENCE_LOGICAL_OR, /* GCC_JIT_BINARY_OP_LOGICAL_OR */
+  PRECEDENCE_SHIFT, /* GCC_JIT_BINARY_OP_LSHIFT */
+  PRECEDENCE_SHIFT, /* GCC_JIT_BINARY_OP_RSHIFT */
+};
+} /* namespace recording */
+
+enum recording::precedence
+recording::binary_op::get_precedence () const
+{
+  return binary_op_precedence[m_op];
+}
+
 /* The implementation of class gcc::jit::recording::comparison.  */
 
 /* Implementation of recording::memento::make_debug_string for
@@ -4313,11 +4396,12 @@ static const char * const comparison_strings[] =
 recording::string *
 recording::comparison::make_debug_string ()
 {
+  enum precedence prec = get_precedence ();
   return string::from_printf (m_ctxt,
 			      "%s %s %s",
-			      m_a->get_debug_string (),
+			      m_a->get_debug_string_parens (prec),
 			      comparison_strings[m_op],
-			      m_b->get_debug_string ());
+			      m_b->get_debug_string_parens (prec));
 }
 
 /* A table of enum gcc_jit_comparison values expressed in string
@@ -4375,6 +4459,25 @@ recording::comparison::visit_children (rvalue_visitor *v)
   v->visit (m_b);
 }
 
+namespace recording {
+static const enum precedence comparison_precedence[] =
+{
+  PRECEDENCE_EQUALITY, /* GCC_JIT_COMPARISON_EQ */
+  PRECEDENCE_EQUALITY, /* GCC_JIT_COMPARISON_NE */
+
+  PRECEDENCE_RELATIONAL,  /* GCC_JIT_COMPARISON_LT */
+  PRECEDENCE_RELATIONAL, /* GCC_JIT_COMPARISON_LE */
+  PRECEDENCE_RELATIONAL,  /* GCC_JIT_COMPARISON_GT */
+  PRECEDENCE_RELATIONAL, /* GCC_JIT_COMPARISON_GE */
+};
+} /* namespace recording */
+
+enum recording::precedence
+recording::comparison::get_precedence () const
+{
+  return comparison_precedence[m_op];
+}
+
 /* Implementation of pure virtual hook recording::memento::replay_into
    for recording::cast.  */
 
@@ -4400,10 +4503,11 @@ recording::cast::visit_children (rvalue_visitor *v)
 recording::string *
 recording::cast::make_debug_string ()
 {
+  enum precedence prec = get_precedence ();
   return string::from_printf (m_ctxt,
 			      "(%s)%s",
 			      get_type ()->get_debug_string (),
-			      m_rvalue->get_debug_string ());
+			      m_rvalue->get_debug_string_parens (prec));
 }
 
 /* Implementation of recording::memento::write_reproducer for casts.  */
@@ -4473,12 +4577,13 @@ recording::call::visit_children (rvalue_visitor *v)
 recording::string *
 recording::call::make_debug_string ()
 {
+  enum precedence prec = get_precedence ();
   /* First, build a buffer for the arguments.  */
   /* Calculate length of said buffer.  */
   size_t sz = 1; /* nil terminator */
   for (unsigned i = 0; i< m_args.length (); i++)
     {
-      sz += strlen (m_args[i]->get_debug_string ());
+      sz += strlen (m_args[i]->get_debug_string_parens (prec));
       sz += 2; /* ", " separator */
     }
 
@@ -4488,8 +4593,8 @@ recording::call::make_debug_string ()
 
   for (unsigned i = 0; i< m_args.length (); i++)
     {
-      strcpy (argbuf + len, m_args[i]->get_debug_string ());
-      len += strlen (m_args[i]->get_debug_string ());
+      strcpy (argbuf + len, m_args[i]->get_debug_string_parens (prec));
+      len += strlen (m_args[i]->get_debug_string_parens (prec));
       if (i + 1 < m_args.length ())
 	{
 	  strcpy (argbuf + len, ", ");
@@ -4586,12 +4691,13 @@ recording::call_through_ptr::visit_children (rvalue_visitor *v)
 recording::string *
 recording::call_through_ptr::make_debug_string ()
 {
+  enum precedence prec = get_precedence ();
   /* First, build a buffer for the arguments.  */
   /* Calculate length of said buffer.  */
   size_t sz = 1; /* nil terminator */
   for (unsigned i = 0; i< m_args.length (); i++)
     {
-      sz += strlen (m_args[i]->get_debug_string ());
+      sz += strlen (m_args[i]->get_debug_string_parens (prec));
       sz += 2; /* ", " separator */
     }
 
@@ -4601,8 +4707,8 @@ recording::call_through_ptr::make_debug_string ()
 
   for (unsigned i = 0; i< m_args.length (); i++)
     {
-      strcpy (argbuf + len, m_args[i]->get_debug_string ());
-      len += strlen (m_args[i]->get_debug_string ());
+      strcpy (argbuf + len, m_args[i]->get_debug_string_parens (prec));
+      len += strlen (m_args[i]->get_debug_string_parens (prec));
       if (i + 1 < m_args.length ())
 	{
 	  strcpy (argbuf + len, ", ");
@@ -4614,7 +4720,7 @@ recording::call_through_ptr::make_debug_string ()
   /* ...and use it to get the string for the call as a whole.  */
   string *result = string::from_printf (m_ctxt,
 					"%s (%s)",
-					m_fn_ptr->get_debug_string (),
+					m_fn_ptr->get_debug_string_parens (prec),
 					argbuf);
 
   delete[] argbuf;
@@ -4680,10 +4786,11 @@ recording::array_access::visit_children (rvalue_visitor *v)
 recording::string *
 recording::array_access::make_debug_string ()
 {
+  enum precedence prec = get_precedence ();
   return string::from_printf (m_ctxt,
 			      "%s[%s]",
-			      m_ptr->get_debug_string (),
-			      m_index->get_debug_string ());
+			      m_ptr->get_debug_string_parens (prec),
+			      m_index->get_debug_string_parens (prec));
 }
 
 /* Implementation of recording::memento::write_reproducer for
@@ -4735,9 +4842,10 @@ recording::access_field_of_lvalue::visit_children (rvalue_visitor *v)
 recording::string *
 recording::access_field_of_lvalue::make_debug_string ()
 {
+  enum precedence prec = get_precedence ();
   return string::from_printf (m_ctxt,
 			      "%s.%s",
-			      m_lvalue->get_debug_string (),
+			      m_lvalue->get_debug_string_parens (prec),
 			      m_field->get_debug_string ());
 }
 
@@ -4787,9 +4895,10 @@ recording::access_field_rvalue::visit_children (rvalue_visitor *v)
 recording::string *
 recording::access_field_rvalue::make_debug_string ()
 {
+  enum precedence prec = get_precedence ();
   return string::from_printf (m_ctxt,
 			      "%s.%s",
-			      m_rvalue->get_debug_string (),
+			      m_rvalue->get_debug_string_parens (prec),
 			      m_field->get_debug_string ());
 }
 
@@ -4840,9 +4949,10 @@ recording::dereference_field_rvalue::visit_children (rvalue_visitor *v)
 recording::string *
 recording::dereference_field_rvalue::make_debug_string ()
 {
+  enum precedence prec = get_precedence ();
   return string::from_printf (m_ctxt,
 			      "%s->%s",
-			      m_rvalue->get_debug_string (),
+			      m_rvalue->get_debug_string_parens (prec),
 			      m_field->get_debug_string ());
 }
 
@@ -4891,9 +5001,10 @@ recording::dereference_rvalue::visit_children (rvalue_visitor *v)
 recording::string *
 recording::dereference_rvalue::make_debug_string ()
 {
+  enum precedence prec = get_precedence ();
   return string::from_printf (m_ctxt,
 			      "*%s",
-			      m_rvalue->get_debug_string ());
+			      m_rvalue->get_debug_string_parens (prec));
 }
 
 /* Implementation of recording::memento::write_reproducer for
@@ -4939,9 +5050,10 @@ recording::get_address_of_lvalue::visit_children (rvalue_visitor *v)
 recording::string *
 recording::get_address_of_lvalue::make_debug_string ()
 {
+  enum precedence prec = get_precedence ();
   return string::from_printf (m_ctxt,
 			      "&%s",
-			      m_lvalue->get_debug_string ());
+			      m_lvalue->get_debug_string_parens (prec));
 }
 
 /* Implementation of recording::memento::write_reproducer for
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 439e7ce..d3170fe 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -858,6 +858,27 @@ class rvalue_visitor
   virtual void visit (rvalue *rvalue) = 0;
 };
 
+/* When generating debug strings for rvalues we mimic C, so we need to
+   mimic C's precedence levels when handling compound expressions.
+   These are in order from strongest precedence to weakest.  */
+enum precedence
+{
+  PRECEDENCE_PRIMARY,
+  PRECEDENCE_POSTFIX,
+  PRECEDENCE_UNARY,
+  PRECEDENCE_CAST,
+  PRECEDENCE_MULTIPLICATIVE,
+  PRECEDENCE_ADDITIVE,
+  PRECEDENCE_SHIFT,
+  PRECEDENCE_RELATIONAL,
+  PRECEDENCE_EQUALITY,
+  PRECEDENCE_BITWISE_AND,
+  PRECEDENCE_BITWISE_XOR,
+  PRECEDENCE_BITWISE_IOR,
+  PRECEDENCE_LOGICAL_AND,
+  PRECEDENCE_LOGICAL_OR
+};
+
 class rvalue : public memento
 {
 public:
@@ -867,7 +888,8 @@ public:
   : memento (ctxt),
     m_loc (loc),
     m_type (type_),
-    m_scope (NULL)
+    m_scope (NULL),
+    m_parenthesized_string (NULL)
   {
     gcc_assert (type_);
   }
@@ -909,12 +931,20 @@ public:
 
   virtual const char *access_as_rvalue (reproducer &r);
 
+  /* Get the debug string, wrapped in parentheses.  */
+  const char *
+  get_debug_string_parens (enum precedence outer_prec);
+
+private:
+  virtual enum precedence get_precedence () const = 0;
+
 protected:
   location *m_loc;
   type *m_type;
 
  private:
   function *m_scope; /* NULL for globals, non-NULL for locals/params */
+  string *m_parenthesized_string;
 };
 
 class lvalue : public rvalue
@@ -977,6 +1007,7 @@ public:
 private:
   string * make_debug_string () { return m_name; }
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_PRIMARY; }
 
 private:
   string *m_name;
@@ -1161,6 +1192,7 @@ public:
 private:
   string * make_debug_string () { return m_name; }
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_PRIMARY; }
 
 private:
   enum gcc_jit_global_kind m_kind;
@@ -1185,6 +1217,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_PRIMARY; }
 
 private:
   HOST_TYPE m_value;
@@ -1206,6 +1239,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_PRIMARY; }
 
 private:
   string *m_value;
@@ -1231,6 +1265,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const {return PRECEDENCE_UNARY;}
 
 private:
   enum gcc_jit_unary_op m_op;
@@ -1257,6 +1292,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const;
 
 private:
   enum gcc_jit_binary_op m_op;
@@ -1284,6 +1320,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const;
 
 private:
   enum gcc_jit_comparison m_op;
@@ -1309,6 +1346,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_CAST; }
 
 private:
   rvalue *m_rvalue;
@@ -1330,6 +1368,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_POSTFIX; }
 
 private:
   function *m_func;
@@ -1352,6 +1391,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_POSTFIX; }
 
 private:
   rvalue *m_fn_ptr;
@@ -1377,6 +1417,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_POSTFIX; }
 
 private:
   rvalue *m_ptr;
@@ -1402,6 +1443,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_POSTFIX; }
 
 private:
   lvalue *m_lvalue;
@@ -1427,6 +1469,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_POSTFIX; }
 
 private:
   rvalue *m_rvalue;
@@ -1452,6 +1495,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_POSTFIX; }
 
 private:
   rvalue *m_rvalue;
@@ -1474,6 +1518,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_UNARY; }
 
 private:
   rvalue *m_rvalue;
@@ -1496,6 +1541,7 @@ public:
 private:
   string * make_debug_string ();
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_UNARY; }
 
 private:
   lvalue *m_lvalue;
@@ -1521,6 +1567,7 @@ public:
 private:
   string * make_debug_string () { return m_name; }
   void write_reproducer (reproducer &r);
+  enum precedence get_precedence () const { return PRECEDENCE_PRIMARY; }
 
 private:
   function *m_func;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 82ce736..f0000cc 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -64,6 +64,13 @@
 #undef create_code
 #undef verify_code
 
+/* test-debug-strings.c */
+#define create_code create_code_debug_strings
+#define verify_code verify_code_debug_strings
+#include "test-debug-strings.c"
+#undef create_code
+#undef verify_code
+
 /* test-dot-product.c */
 #define create_code create_code_dot_product
 #define verify_code verify_code_dot_product
diff --git a/gcc/testsuite/jit.dg/test-debug-strings.c b/gcc/testsuite/jit.dg/test-debug-strings.c
new file mode 100644
index 0000000..e515a17
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-debug-strings.c
@@ -0,0 +1,190 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+/* Build various compound expressions, and verify that they have sane debug
+   strings.  */
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+  /* Make a singly-linked list type:
+      struct node
+      {
+       struct node *next;
+       int value;
+      };
+  */
+  gcc_jit_type *t_int =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+  gcc_jit_struct *t_node =
+    gcc_jit_context_new_opaque_struct (ctxt, NULL, "node");
+  gcc_jit_type *t_node_ptr =
+    gcc_jit_type_get_pointer (gcc_jit_struct_as_type (t_node));
+  gcc_jit_field *f_next =
+    gcc_jit_context_new_field (ctxt, NULL, t_node_ptr, "next");
+  gcc_jit_field *f_value =
+    gcc_jit_context_new_field (ctxt, NULL, t_int, "value");
+  gcc_jit_field *fields[] = {f_next, f_value};
+  gcc_jit_struct_set_fields (t_node, NULL, 2, fields);
+
+  /* Create a dummy function so that we have locals/params to build
+     expressions with.  */
+  gcc_jit_type *t_void =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID);
+  gcc_jit_function *fn =
+    gcc_jit_context_new_function (ctxt, NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  t_void,
+				  "test_debug_strings",
+				  0, NULL, 0);
+  gcc_jit_rvalue *ptr =
+    gcc_jit_lvalue_as_rvalue (
+      gcc_jit_function_new_local (fn,
+				  NULL,
+				  t_node_ptr,
+				  "ptr"));
+  gcc_jit_rvalue *a =
+    gcc_jit_lvalue_as_rvalue (
+      gcc_jit_function_new_local (fn, NULL, t_int, "a"));
+  gcc_jit_rvalue *b =
+    gcc_jit_lvalue_as_rvalue (
+      gcc_jit_function_new_local (fn, NULL, t_int, "b"));
+  gcc_jit_rvalue *c =
+    gcc_jit_lvalue_as_rvalue (
+      gcc_jit_function_new_local (fn, NULL, t_int, "c"));
+  gcc_jit_rvalue *d =
+    gcc_jit_lvalue_as_rvalue (
+      gcc_jit_function_new_local (fn, NULL, t_int, "d"));
+
+#define CHECK_RVALUE_DEBUG_STRING(RVALUE, EXPECTED) \
+  CHECK_STRING_VALUE ( \
+    gcc_jit_object_get_debug_string (gcc_jit_rvalue_as_object (RVALUE)), \
+    (EXPECTED))
+
+#define CHECK_LVALUE_DEBUG_STRING(LVALUE, EXPECTED) \
+  CHECK_STRING_VALUE ( \
+    gcc_jit_object_get_debug_string (gcc_jit_lvalue_as_object (LVALUE)), \
+    (EXPECTED))
+
+  /* Verify various simple compound expressions.  */
+  {
+    CHECK_RVALUE_DEBUG_STRING (ptr, "ptr");
+
+    gcc_jit_lvalue *deref =
+      gcc_jit_rvalue_dereference_field (ptr,
+					NULL,
+					f_value);
+    CHECK_LVALUE_DEBUG_STRING (deref, "ptr->value");
+
+    gcc_jit_rvalue *deref_as_rvalue = gcc_jit_lvalue_as_rvalue (deref);
+
+#define BINOP(OP, A, B) \
+    gcc_jit_context_new_binary_op (ctxt, NULL, \
+				   GCC_JIT_BINARY_OP_##OP, t_int, (A), (B))
+#define COMPARISON(OP, A, B) \
+    gcc_jit_context_new_comparison (ctxt, NULL, \
+				    GCC_JIT_COMPARISON_##OP,(A), (B))
+
+    CHECK_RVALUE_DEBUG_STRING (
+      BINOP (PLUS, deref_as_rvalue, deref_as_rvalue),
+      "ptr->value + ptr->value");
+    CHECK_RVALUE_DEBUG_STRING (
+      BINOP (MULT, deref_as_rvalue, deref_as_rvalue),
+      "ptr->value * ptr->value");
+
+   /* Multiplication has higher precedence in C than addition, so this
+       dump shouldn't contain parentheses.  */
+    CHECK_RVALUE_DEBUG_STRING (
+      BINOP (PLUS,
+	     BINOP (MULT, a, b),
+	     BINOP (MULT, c, d)),
+      "a * b + c * d");
+
+    /* ...but this one should.  */
+    CHECK_RVALUE_DEBUG_STRING (
+      BINOP (MULT,
+	     BINOP (PLUS, a, b),
+	     BINOP (PLUS, c, d)),
+      "(a + b) * (c + d)");
+
+    /* Equal precedences don't need parentheses.  */
+    CHECK_RVALUE_DEBUG_STRING (
+      BINOP (MULT,
+	     BINOP (MULT, a, b),
+	     BINOP (MULT, c, d)),
+      "a * b * c * d");
+
+    /* Comparisons and logical ops.  */
+    CHECK_RVALUE_DEBUG_STRING (
+      COMPARISON (LT, a, b),
+      "a < b");
+
+    CHECK_RVALUE_DEBUG_STRING (
+      BINOP (LOGICAL_AND,
+	     COMPARISON (LT, a, b),
+	     COMPARISON (GT, c, d)),
+      "a < b && c > d");
+
+    CHECK_RVALUE_DEBUG_STRING (
+      BINOP (LOGICAL_AND,
+	     BINOP (LOGICAL_OR,
+		    COMPARISON (LT, a, b),
+		    COMPARISON (LT, a, c)),
+	     BINOP (LOGICAL_OR,
+		    COMPARISON (GT, d, b),
+		    COMPARISON (GT, d, c))),
+      "(a < b || a < c) && (d > b || d > c)");
+
+    CHECK_RVALUE_DEBUG_STRING (
+      BINOP (LOGICAL_OR,
+	     BINOP (LOGICAL_AND,
+		    COMPARISON (LT, a, b),
+		    COMPARISON (LT, a, c)),
+	     BINOP (LOGICAL_AND,
+		    COMPARISON (GT, d, b),
+		    COMPARISON (GT, d, c))),
+      "a < b && a < c || d > b && d > c");
+
+#undef BINOP
+#undef COMPARISON
+  }
+
+  /* PR jit/66539 "Missing parentheses in jit dumps".
+     Construct the equivalent of
+       ((cast)ptr->next)->next
+     and verify that the appropriate parentheses appear in the debug
+     string.   */
+  {
+    /* "ptr->next". */
+    gcc_jit_lvalue *inner_deref =
+      gcc_jit_rvalue_dereference_field (ptr,
+					NULL,
+					f_next);
+    /* "((node *)ptr->next)"; the cast is redundant, purely
+       to exercise dumping.  */
+    gcc_jit_rvalue *test_cast =
+      gcc_jit_context_new_cast (ctxt, NULL,
+				gcc_jit_lvalue_as_rvalue (inner_deref),
+				t_node_ptr);
+    /* "((node *)ptr->next)->next".  */
+    gcc_jit_lvalue *outer_deref =
+      gcc_jit_rvalue_dereference_field (test_cast, /* gcc_jit_rvalue *ptr */
+					NULL, /* gcc_jit_location *loc */
+					f_next); /* gcc_jit_field *field */
+    CHECK_LVALUE_DEBUG_STRING (outer_deref,
+			       "((struct node *)ptr->next)->next");
+  }
+
+#undef CHECK_LVALUE_DEBUG_STRING
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  CHECK_NON_NULL (result);
+  /* We don't actually build any functions above;
+     nothing more to verify.  */
+}
diff --git a/gcc/testsuite/jit.dg/test-quadratic.c b/gcc/testsuite/jit.dg/test-quadratic.c
index 715174c..d83a4fb 100644
--- a/gcc/testsuite/jit.dg/test-quadratic.c
+++ b/gcc/testsuite/jit.dg/test-quadratic.c
@@ -176,16 +176,8 @@ make_calc_discriminant (struct quadratic_test *testcase)
 	  gcc_jit_param_as_rvalue (param_q),
 	  NULL, testcase->c));
 
-  gcc_jit_block_add_assignment (
-    blk, NULL,
-
-    /* q->discriminant =...  */
-    gcc_jit_rvalue_dereference_field (
-      gcc_jit_param_as_rvalue (param_q),
-      NULL,
-      testcase->discriminant),
-
-    /* (q->b * q->b) - (4 * q->a * q->c) */
+  /* (q->b * q->b) - (4 * q->a * q->c) */
+  gcc_jit_rvalue *rhs =
     gcc_jit_context_new_binary_op (
       testcase->ctxt, NULL,
       GCC_JIT_BINARY_OP_MINUS,
@@ -213,7 +205,21 @@ make_calc_discriminant (struct quadratic_test *testcase)
 	  testcase->ctxt, NULL,
 	  GCC_JIT_BINARY_OP_MULT,
 	  testcase->numeric_type,
-	  q_a, q_c)))); /* end of gcc_jit_function_add_assignment call.  */
+	  q_a, q_c)));
+
+  CHECK_STRING_VALUE (
+     gcc_jit_object_get_debug_string (gcc_jit_rvalue_as_object (rhs)),
+     "q->b * q->b - (double)4 * q->a * q->c");
+
+  gcc_jit_block_add_assignment (
+    blk, NULL,
+
+    /* q->discriminant =...  */
+    gcc_jit_rvalue_dereference_field (
+      gcc_jit_param_as_rvalue (param_q),
+      NULL,
+      testcase->discriminant),
+    rhs);
 
   gcc_jit_block_end_with_void_return (blk, NULL);
 }
-- 
1.8.5.3

Re: The output of gcc_jit_context_dump_to_file() is missing parenthesis in expressions involving casts

[David Malcolm]

On Sun, 2015-06-14 at 20:11 +0100, Dibyendu Majumdar wrote:
> It seems that in some cases the output is not showing parenthesis
> where it should.

Yes, this is a bug.

The dumping fns are somewhat hacky; they have no concept of C precedence
order, and are probably misleading on some expressions.  Sorry.

I've filed it as:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66539

> For instance:
> 
>  if ((struct ravi_LClosure *)L->ci->func->p->sizep > (int)0)
> 
> 
> The cast is to be applied to L->ci->func - so this expression should
> be in parenthesis.
> 
>   gcc_jit_rvalue *rvalue__struct_ravi_LClosure___L__ci__func_0xae4e10 =
>     gcc_jit_context_new_cast (ctxt_0xae4350,
>                               NULL, /* gcc_jit_location *loc */
>                               gcc_jit_lvalue_as_rvalue
> (lvalue_L__ci__func_0xae4b90), /* gcc_jit_rvalue *rvalue */
> 
>   gcc_jit_lvalue *lvalue__struct_ravi_LClosure___L__ci__func__p_0xae4fd0=
>     gcc_jit_rvalue_dereference_field
> (rvalue__struct_ravi_LClosure___L__ci__func_0xae4e10, /*
> gcc_jit_rvalue *ptr */
>                                       NULL, /* gcc_jit_location *loc */
>                                       field_p_0xacf1d0); /*
> gcc_jit_field *field */
> 
>   gcc_jit_lvalue *lvalue__struct_ravi_LClosure___L__ci__func__p__k_0xae5020=
>     gcc_jit_rvalue_dereference_field (gcc_jit_lvalue_as_rvalue
> (lvalue__struct_ravi_LClosure___L__ci__func__p_0xae4fd0), /*
> gcc_jit_rvalue *ptr */
>                                       NULL, /* gcc_jit_location *loc */
>                                       field_k_0xacdc10); /*
> gcc_jit_field *field */

Re: The output of gcc_jit_context_dump_to_file() is missing parenthesis in expressions involving casts

[David Malcolm]

On Sun, 2015-06-14 at 20:42 -0400, David Malcolm wrote:
> On Sun, 2015-06-14 at 20:11 +0100, Dibyendu Majumdar wrote:
> > It seems that in some cases the output is not showing parenthesis
> > where it should.
> 
> Yes, this is a bug.
> 
> The dumping fns are somewhat hacky; they have no concept of C precedence
> order, and are probably misleading on some expressions.  Sorry.
> 
> I've filed it as:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66539

(snip)

This should be fixed now for gcc trunk (for GCC 6) as of r224531;
gcc_jit_get_debug_string should now add parentheses where needed,
mimicking C's precedence order. 

If after updating to r224531 you still see any expressions where it gets
it wrong, please let me know.

Dave

The output of gcc_jit_context_dump_to_file() is missing parenthesis in expressions involving casts

[Dibyendu Majumdar]

It seems that in some cases the output is not showing parenthesis
where it should.

For instance:

 if ((struct ravi_LClosure *)L->ci->func->p->sizep > (int)0)


The cast is to be applied to L->ci->func - so this expression should
be in parenthesis.

  gcc_jit_rvalue *rvalue__struct_ravi_LClosure___L__ci__func_0xae4e10 =
    gcc_jit_context_new_cast (ctxt_0xae4350,
                              NULL, /* gcc_jit_location *loc */
                              gcc_jit_lvalue_as_rvalue
(lvalue_L__ci__func_0xae4b90), /* gcc_jit_rvalue *rvalue */

  gcc_jit_lvalue *lvalue__struct_ravi_LClosure___L__ci__func__p_0xae4fd0=
    gcc_jit_rvalue_dereference_field
(rvalue__struct_ravi_LClosure___L__ci__func_0xae4e10, /*
gcc_jit_rvalue *ptr */
                                      NULL, /* gcc_jit_location *loc */
                                      field_p_0xacf1d0); /*
gcc_jit_field *field */

  gcc_jit_lvalue *lvalue__struct_ravi_LClosure___L__ci__func__p__k_0xae5020=
    gcc_jit_rvalue_dereference_field (gcc_jit_lvalue_as_rvalue
(lvalue__struct_ravi_LClosure___L__ci__func__p_0xae4fd0), /*
gcc_jit_rvalue *ptr */
                                      NULL, /* gcc_jit_location *loc */
                                      field_k_0xacdc10); /*
gcc_jit_field *field */