Re: [PATCH][RFC] Re-work GIMPLE checking to be gimple class friendly

[Richard Biener <rguenther at suse dot de>]

On Mon, 27 Jul 2015, Richard Biener wrote:

> On Mon, 27 Jul 2015, Richard Biener wrote:
> 
> > 
> > I noticed that the code we generate for a simple gimple_assign_rhs1 (stmt)
> > is quite bad as we have two checking pieces.  The implementation is now
> > 
> >  static inline tree
> >  gimple_assign_rhs1 (const_gimple gs)
> >  {
> >    GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
> >    return gimple_op (gs, 1);
> >  }
> > 
> > and the hidden checking is due to gimple_op being
> > 
> > static inline tree *
> > gimple_ops (gimple gs)
> > {
> >   size_t off;
> > 
> >   /* All the tuples have their operand vector at the very bottom
> >      of the structure.  Note that those structures that do not
> >      have an operand vector have a zero offset.  */
> >   off = gimple_ops_offset_[gimple_statement_structure (gs)];
> >   gcc_gimple_checking_assert (off != 0);
> > 
> >   return (tree *) ((char *) gs + off);
> > 
> > (ugly in itself considering that we just verified we have a gassign
> > which has an operand vector as member...).  gimple_op incurs two
> > additional memory reference to compute gimple_statement_structure
> > and gimple_ops_offset_ plus the checking bits.
> > 
> > The simplest thing would be to use
> > 
> >   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
> >   return as_a <const gassign *> (gs)->op[1];
> > 
> > but that's not optimal either because we check we have a gassign
> > again (not optimized in stage1).  So I decided to invent a fancy
> > as_a which reports errors similar to GIMPLE_CHECK and makes the
> > code look like
> > 
> >   const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
> >   return ass->op[1];
> > 
> > for some reason I needed to overload is_a_helper for const_gimple
> > (I thought the is_a machinery would transparently support qualified
> > types?).
> > 
> > I'm missing a fallback for !ENABLE_GIMPLE_CHECKING as well
> > as an overload for 'gimple' I guess.  I just changed
> > gimple_assign_rhs1 for now.
> > 
> > So this is a RFC.
> > 
> > I understand in the end the whole GCC may use gassign everywhere
> > we access gimple_assign_rhs1 but I don't see this happening soon
> > and this simple proof-of-concept patch already reduces unoptimized
> > text size of gimple-match.c from 836080 to 836359 (too bad) and
> > optimized from 585731 to 193190 bytes (yay!) on x86_64 using trunk.
> > Some inlining must go very differently - we just have 544 calls to
> > gimple_assign_rhs1 in gimple-match.c.  .optimizes shows all calls
> > remaining as
> > 
> >   <bb 89>:
> >   _ZL18gimple_assign_rhs1PK21gimple_statement_base.part.32 (def_stmt_47);
> > 
> > which is
> > 
> > tree_node* gimple_assign_rhs1(const_gimple) (const struct 
> > gimple_statement_base * gs)
> > {
> >   <bb 2>:
> >   gimple_check_failed (gs_1(D), 
> > &"/space/rguenther/tramp3d/trunk/gcc/gimple.h"[0], 2381, 
> > &"gimple_assign_rhs1"[0], 6, 0);
> > 
> > }
> > 
> > so eventually we just do a better job with profile estimation.
> 
> Ok, numbers are off because of a typo I introduced late.
> 
> > +  if (ret)
> > +    gimple_check_failed (gs, file, line, fun, T()->code_, ERROR_MARK);
> 
> should be if (!ret) of course ...
> 
> Optimized code size after the patch is 588878 (we still avoid
> a lot of code and the checking fails are still split out).  Not sure
> where the code size increase is from.  Unoptimized code size
> after the patch is 836233.

And here is a more complete variant (covering all op accesses of
gimple assigns).  stage2 cc1plus size drops from

   text    data     bss     dec     hex filename
24970991          73776 1392952 26437719        1936857 
/abuild/rguenther/obj2/prev-gcc/cc1plus

to

  text    data     bss     dec     hex filename
24809991          69608 1388536 26268135        190d1e7 
../obj/prev-gcc/cc1plus

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Thanks,
Richard.

2015-07-27  Richard Biener  <rguenther@suse.de>

	* gimple.h (GIMPLE_CHECK2): New inline template function.
	(gassign::code_): New constant static member.
	(is_a_helper<const gassign *>): Add.
	(gimple_assign_lhs): Use GIMPLE_CHECK2 and a cheaper way
	to access the operand.
	(gimple_assign_lhs_ptr): Likewise.
	(gimple_assign_set_lhs): Likewise.
	(gimple_assign_rhs1, gimple_assign_rhs1_ptr, gimple_assign_set_rhs1):
	Likewise.
	(gimple_assign_rhs2, gimple_assign_rhs2_ptr, gimple_assign_set_rhs2):
	Likewise.
	(gimple_assign_rhs3, gimple_assign_rhs3_ptr, gimple_assign_set_rhs3):
	Likewise.
	(gimple_assign_rhs_code): Likewise.
	* gimple.c (gassign::code_): Define.


Index: gcc/gimple.h
===================================================================
--- gcc/gimple.h	(revision 226240)
+++ gcc/gimple.h	(working copy)
@@ -51,9 +51,57 @@ extern void gimple_check_failed (const_g
       gimple_check_failed (__gs, __FILE__, __LINE__, __FUNCTION__,	\
 	  		   (CODE), ERROR_MARK);				\
   } while (0)
+template <typename T>
+static inline T
+GIMPLE_CHECK2(const_gimple gs,
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)
+	      const char *file = __builtin_FILE (),
+	      int line = __builtin_LINE (),
+	      const char *fun = __builtin_FUNCTION ())
+#else
+	      const char *file = __FILE__,
+	      int line = __LINE__,
+	      const char *fun = NULL)
+#endif
+{
+  T ret = dyn_cast <T> (gs);
+  if (!ret)
+    gimple_check_failed (gs, file, line, fun, T()->code_, ERROR_MARK);
+  return ret;
+}
+template <typename T>
+static inline T
+GIMPLE_CHECK2(gimple gs,
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)
+	      const char *file = __builtin_FILE (),
+	      int line = __builtin_LINE (),
+	      const char *fun = __builtin_FUNCTION ())
+#else
+	      const char *file = __FILE__,
+	      int line = __LINE__,
+	      const char *fun = NULL)
+#endif
+{
+  T ret = dyn_cast <T> (gs);
+  if (!ret)
+    gimple_check_failed (gs, file, line, fun, T()->code_, ERROR_MARK);
+  return ret;
+}
 #else  /* not ENABLE_GIMPLE_CHECKING  */
 #define gcc_gimple_checking_assert(EXPR) ((void)(0 && (EXPR)))
 #define GIMPLE_CHECK(GS, CODE)			(void)0
+template <typename T>
+static inline T
+GIMPLE_CHECK2(gimple gs)
+{
+  return as_a <T> (gs);
+}
+template <typename T>
+static inline T
+GIMPLE_CHECK2(const_gimple gs)
+{
+  return as_a <T> (gs);
+}
 #endif
 
 /* Class of GIMPLE expressions suitable for the RHS of assignments.  See
@@ -832,6 +880,7 @@ struct GTY((tag("GSS_WITH_OPS")))
 struct GTY((tag("GSS_WITH_MEM_OPS")))
   gassign : public gimple_statement_with_memory_ops
 {
+  static const enum gimple_code code_ = GIMPLE_ASSIGN;
   /* no additional fields; this uses the layout for GSS_WITH_MEM_OPS. */
 };
 
@@ -864,6 +913,14 @@ is_a_helper <gassign *>::test (gimple gs
 template <>
 template <>
 inline bool
+is_a_helper <const gassign *>::test (const_gimple gs)
+{
+  return gs->code == GIMPLE_ASSIGN;
+}
+
+template <>
+template <>
+inline bool
 is_a_helper <gbind *>::test (gimple gs)
 {
   return gs->code == GIMPLE_BIND;
@@ -2326,8 +2383,8 @@ get_gimple_rhs_class (enum tree_code cod
 static inline tree
 gimple_assign_lhs (const_gimple gs)
 {
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
-  return gimple_op (gs, 0);
+  const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
+  return ass->op[0];
 }
 
 
@@ -2336,8 +2393,8 @@ gimple_assign_lhs (const_gimple gs)
 static inline tree *
 gimple_assign_lhs_ptr (const_gimple gs)
 {
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
-  return gimple_op_ptr (gs, 0);
+  const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
+  return const_cast<tree *> (&ass->op[0]);
 }
 
 
@@ -2346,8 +2403,8 @@ gimple_assign_lhs_ptr (const_gimple gs)
 static inline void
 gimple_assign_set_lhs (gimple gs, tree lhs)
 {
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
-  gimple_set_op (gs, 0, lhs);
+  gassign *ass = GIMPLE_CHECK2<gassign *> (gs);
+  ass->op[0] = lhs;
 
   if (lhs && TREE_CODE (lhs) == SSA_NAME)
     SSA_NAME_DEF_STMT (lhs) = gs;
@@ -2359,8 +2416,8 @@ gimple_assign_set_lhs (gimple gs, tree l
 static inline tree
 gimple_assign_rhs1 (const_gimple gs)
 {
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
-  return gimple_op (gs, 1);
+  const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
+  return ass->op[1];
 }
 
 
@@ -2370,8 +2427,8 @@ gimple_assign_rhs1 (const_gimple gs)
 static inline tree *
 gimple_assign_rhs1_ptr (const_gimple gs)
 {
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
-  return gimple_op_ptr (gs, 1);
+  const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
+  return const_cast<tree *> (&ass->op[1]);
 }
 
 /* Set RHS to be the first operand on the RHS of assignment statement GS.  */
@@ -2379,9 +2436,8 @@ gimple_assign_rhs1_ptr (const_gimple gs)
 static inline void
 gimple_assign_set_rhs1 (gimple gs, tree rhs)
 {
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
-
-  gimple_set_op (gs, 1, rhs);
+  gassign *ass = GIMPLE_CHECK2<gassign *> (gs);
+  ass->op[1] = rhs;
 }
 
 
@@ -2391,10 +2447,9 @@ gimple_assign_set_rhs1 (gimple gs, tree
 static inline tree
 gimple_assign_rhs2 (const_gimple gs)
 {
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
-
+  const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
   if (gimple_num_ops (gs) >= 3)
-    return gimple_op (gs, 2);
+    return ass->op[2];
   else
     return NULL_TREE;
 }
@@ -2406,8 +2461,9 @@ gimple_assign_rhs2 (const_gimple gs)
 static inline tree *
 gimple_assign_rhs2_ptr (const_gimple gs)
 {
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
-  return gimple_op_ptr (gs, 2);
+  const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
+  gcc_gimple_checking_assert (gimple_num_ops (gs) >= 3);
+  return const_cast<tree *> (&ass->op[2]);
 }
 
 
@@ -2416,9 +2472,9 @@ gimple_assign_rhs2_ptr (const_gimple gs)
 static inline void
 gimple_assign_set_rhs2 (gimple gs, tree rhs)
 {
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
-
-  gimple_set_op (gs, 2, rhs);
+  gassign *ass = GIMPLE_CHECK2<gassign *> (gs);
+  gcc_gimple_checking_assert (gimple_num_ops (gs) >= 3);
+  ass->op[2] = rhs;
 }
 
 /* Return the third operand on the RHS of assignment statement GS.
@@ -2427,10 +2483,9 @@ gimple_assign_set_rhs2 (gimple gs, tree
 static inline tree
 gimple_assign_rhs3 (const_gimple gs)
 {
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
-
+  const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
   if (gimple_num_ops (gs) >= 4)
-    return gimple_op (gs, 3);
+    return ass->op[3];
   else
     return NULL_TREE;
 }
@@ -2441,8 +2496,9 @@ gimple_assign_rhs3 (const_gimple gs)
 static inline tree *
 gimple_assign_rhs3_ptr (const_gimple gs)
 {
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
-  return gimple_op_ptr (gs, 3);
+  const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
+  gcc_gimple_checking_assert (gimple_num_ops (gs) >= 4);
+  return const_cast<tree *> (&ass->op[3]);
 }
 
 
@@ -2451,9 +2507,9 @@ gimple_assign_rhs3_ptr (const_gimple gs)
 static inline void
 gimple_assign_set_rhs3 (gimple gs, tree rhs)
 {
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
-
-  gimple_set_op (gs, 3, rhs);
+  gassign *ass = GIMPLE_CHECK2<gassign *> (gs);
+  gcc_gimple_checking_assert (gimple_num_ops (gs) >= 4);
+  ass->op[3] = rhs;
 }
 
 /* A wrapper around 3 operand gimple_assign_set_rhs_with_ops, for callers
@@ -2502,14 +2558,14 @@ static inline enum tree_code
 gimple_assign_rhs_code (const_gimple gs)
 {
   enum tree_code code;
-  GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
+  const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
 
   code = (enum tree_code) gs->subcode;
   /* While we initially set subcode to the TREE_CODE of the rhs for
      GIMPLE_SINGLE_RHS assigns we do not update that subcode to stay
      in sync when we rewrite stmts into SSA form or do SSA propagations.  */
   if (get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS)
-    code = TREE_CODE (gimple_assign_rhs1 (gs));
+    code = TREE_CODE (ass->op[1]);
 
   return code;
 }
Index: gcc/gimple.c
===================================================================
--- gcc/gimple.c	(revision 226240)
+++ gcc/gimple.c	(working copy)
@@ -89,6 +89,10 @@ static const char * const gimple_alloc_k
     "everything else"
 };
 
+/* Static gimple tuple members.  */
+const enum gimple_code gassign::code_;
+
+
 /* Gimple tuple constructors.
    Note: Any constructor taking a ``gimple_seq'' as a parameter, can
    be passed a NULL to start with an empty sequence.  */