[PATCH][RFC] Re-work GIMPLE checking to be gimple class friendly (even better)

Richard Biener rguenther@suse.de
Mon Aug 3 13:39:00 GMT 2015


On Mon, 27 Jul 2015, Richard Biener wrote:

> 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

So we tried to come up with the reason that there are no gassign *
overloads already in place.  And remembered sth about C++ and not
working overload resolution and such.  But experiments show that
all cases we tried are handled well (including const gassign * -> gimple
conversions).  So here is a variant of the patch adding those
overloads and making the gimple variants forwarders with just the
(new and fancy) GIMPLE checking.

I'm leaving this for discussion again but plan to commit it this time
if there is no further response (after Cauldron, that is).

Btw, I wonder what happened to the gimple -> gimple * conversion.

Bootstrap & regtest running on x86_64-unknown-linux-gnu.

Richard.

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

	* gimple.h (remove_pointer): New trait.
	(GIMPLE_CHECK2): New inline template function.
	(gassign::code_): New constant static member.
	(is_a_helper<const gassign *>): Add.
	(gimple_assign_lhs): Use GIMPLE_CHECK2 in the gimple overload
	and forward to a new gassign overload with less checking 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 226490)
--- gcc/gimple.h	(working copy)
*************** enum gimple_code {
*** 37,42 ****
--- 37,46 ----
  extern const char *const gimple_code_name[];
  extern const unsigned char gimple_rhs_class_table[];
  
+ /* Strip the outermost pointer, from tr1/type_traits.  */
+ template<typename T> struct remove_pointer { typedef T type; };
+ template<typename T> struct remove_pointer<T *> { typedef T type; };
+ 
  /* Error out if a gimple tuple is addressed incorrectly.  */
  #if defined ENABLE_GIMPLE_CHECKING
  #define gcc_gimple_checking_assert(EXPR) gcc_assert (EXPR)
*************** extern void gimple_check_failed (const_g
*** 51,59 ****
--- 55,113 ----
        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,
+ 			 remove_pointer<T>::type::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,
+ 			 remove_pointer<T>::type::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
*************** struct GTY((tag("GSS_WITH_OPS")))
*** 832,837 ****
--- 886,892 ----
  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. */
  };
  
*************** is_a_helper <gassign *>::test (gimple gs
*** 864,869 ****
--- 919,932 ----
  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;
*************** get_gimple_rhs_class (enum tree_code cod
*** 2324,2366 ****
  /* Return the LHS of assignment statement GS.  */
  
  static inline tree
  gimple_assign_lhs (const_gimple gs)
  {
!   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
!   return gimple_op (gs, 0);
  }
  
  
  /* Return a pointer to the LHS of assignment statement GS.  */
  
  static inline tree *
  gimple_assign_lhs_ptr (const_gimple gs)
  {
!   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
!   return gimple_op_ptr (gs, 0);
  }
  
  
  /* Set LHS to be the LHS operand of assignment statement GS.  */
  
  static inline void
! gimple_assign_set_lhs (gimple gs, tree lhs)
  {
!   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
!   gimple_set_op (gs, 0, lhs);
  
    if (lhs && TREE_CODE (lhs) == SSA_NAME)
      SSA_NAME_DEF_STMT (lhs) = gs;
  }
  
  
  /* Return the first operand on the RHS of assignment statement GS.  */
  
  static inline tree
  gimple_assign_rhs1 (const_gimple gs)
  {
!   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
!   return gimple_op (gs, 1);
  }
  
  
--- 2387,2453 ----
  /* Return the LHS of assignment statement GS.  */
  
  static inline tree
+ gimple_assign_lhs (const gassign *gs)
+ {
+   return gs->op[0];
+ }
+ 
+ static inline tree
  gimple_assign_lhs (const_gimple gs)
  {
!   const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
!   return gimple_assign_lhs (ass);
  }
  
  
  /* Return a pointer to the LHS of assignment statement GS.  */
  
  static inline tree *
+ gimple_assign_lhs_ptr (const gassign *gs)
+ {
+   return const_cast<tree *> (&gs->op[0]);
+ }
+ 
+ static inline tree *
  gimple_assign_lhs_ptr (const_gimple gs)
  {
!   const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
!   return gimple_assign_lhs_ptr (ass);
  }
  
  
  /* Set LHS to be the LHS operand of assignment statement GS.  */
  
  static inline void
! gimple_assign_set_lhs (gassign *gs, tree lhs)
  {
!   gs->op[0] = lhs;
  
    if (lhs && TREE_CODE (lhs) == SSA_NAME)
      SSA_NAME_DEF_STMT (lhs) = gs;
  }
  
+ static inline void
+ gimple_assign_set_lhs (gimple gs, tree lhs)
+ {
+   gassign *ass = GIMPLE_CHECK2<gassign *> (gs);
+   gimple_assign_set_lhs (ass, lhs);
+ }
+ 
  
  /* Return the first operand on the RHS of assignment statement GS.  */
  
  static inline tree
+ gimple_assign_rhs1 (const gassign *gs)
+ {
+   return gs->op[1];
+ }
+ 
+ static inline tree
  gimple_assign_rhs1 (const_gimple gs)
  {
!   const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
!   return gimple_assign_rhs1 (ass);
  }
  
  
*************** gimple_assign_rhs1 (const_gimple gs)
*** 2368,2387 ****
     statement GS.  */
  
  static inline tree *
  gimple_assign_rhs1_ptr (const_gimple gs)
  {
!   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
!   return gimple_op_ptr (gs, 1);
  }
  
  /* Set RHS to be the first operand on the RHS of assignment statement GS.  */
  
  static inline void
! gimple_assign_set_rhs1 (gimple gs, tree rhs)
  {
!   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
  
!   gimple_set_op (gs, 1, rhs);
  }
  
  
--- 2455,2485 ----
     statement GS.  */
  
  static inline tree *
+ gimple_assign_rhs1_ptr (const gassign *gs)
+ {
+   return const_cast<tree *> (&gs->op[1]);
+ }
+ 
+ static inline tree *
  gimple_assign_rhs1_ptr (const_gimple gs)
  {
!   const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
!   return gimple_assign_rhs1_ptr (ass);
  }
  
  /* Set RHS to be the first operand on the RHS of assignment statement GS.  */
  
  static inline void
! gimple_assign_set_rhs1 (gassign *gs, tree rhs)
  {
!   gs->op[1] = rhs;
! }
  
! static inline void
! gimple_assign_set_rhs1 (gimple gs, tree rhs)
! {
!   gassign *ass = GIMPLE_CHECK2<gassign *> (gs);
!   gimple_assign_set_rhs1 (ass, rhs);
  }
  
  
*************** gimple_assign_set_rhs1 (gimple gs, tree
*** 2389,2461 ****
     If GS does not have two operands, NULL is returned instead.  */
  
  static inline tree
! gimple_assign_rhs2 (const_gimple gs)
  {
-   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
- 
    if (gimple_num_ops (gs) >= 3)
!     return gimple_op (gs, 2);
    else
      return NULL_TREE;
  }
  
  
  /* Return a pointer to the second operand on the RHS of assignment
     statement GS.  */
  
  static inline tree *
  gimple_assign_rhs2_ptr (const_gimple gs)
  {
!   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
!   return gimple_op_ptr (gs, 2);
  }
  
  
  /* Set RHS to be the second operand on the RHS of assignment statement GS.  */
  
  static inline void
! gimple_assign_set_rhs2 (gimple gs, tree rhs)
  {
!   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
  
!   gimple_set_op (gs, 2, rhs);
  }
  
  /* Return the third operand on the RHS of assignment statement GS.
     If GS does not have two operands, NULL is returned instead.  */
  
  static inline tree
! gimple_assign_rhs3 (const_gimple gs)
  {
-   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
- 
    if (gimple_num_ops (gs) >= 4)
!     return gimple_op (gs, 3);
    else
      return NULL_TREE;
  }
  
  /* Return a pointer to the third operand on the RHS of assignment
     statement GS.  */
  
  static inline tree *
  gimple_assign_rhs3_ptr (const_gimple gs)
  {
!   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
!   return gimple_op_ptr (gs, 3);
  }
  
  
  /* Set RHS to be the third operand on the RHS of assignment statement GS.  */
  
  static inline void
! gimple_assign_set_rhs3 (gimple gs, tree rhs)
  {
!   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
  
!   gimple_set_op (gs, 3, rhs);
  }
  
  /* A wrapper around 3 operand gimple_assign_set_rhs_with_ops, for callers
     which expect to see only two operands.  */
  
--- 2487,2590 ----
     If GS does not have two operands, NULL is returned instead.  */
  
  static inline tree
! gimple_assign_rhs2 (const gassign *gs)
  {
    if (gimple_num_ops (gs) >= 3)
!     return gs->op[2];
    else
      return NULL_TREE;
  }
  
+ static inline tree
+ gimple_assign_rhs2 (const_gimple gs)
+ {
+   const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
+   return gimple_assign_rhs2 (ass);
+ }
+ 
  
  /* Return a pointer to the second operand on the RHS of assignment
     statement GS.  */
  
  static inline tree *
+ gimple_assign_rhs2_ptr (const gassign *gs)
+ {
+   gcc_gimple_checking_assert (gimple_num_ops (gs) >= 3);
+   return const_cast<tree *> (&gs->op[2]);
+ }
+ 
+ static inline tree *
  gimple_assign_rhs2_ptr (const_gimple gs)
  {
!   const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
!   return gimple_assign_rhs2_ptr (ass);
  }
  
  
  /* Set RHS to be the second operand on the RHS of assignment statement GS.  */
  
  static inline void
! gimple_assign_set_rhs2 (gassign *gs, tree rhs)
  {
!   gcc_gimple_checking_assert (gimple_num_ops (gs) >= 3);
!   gs->op[2] = rhs;
! }
  
! static inline void
! gimple_assign_set_rhs2 (gimple gs, tree rhs)
! {
!   gassign *ass = GIMPLE_CHECK2<gassign *> (gs);
!   return gimple_assign_set_rhs2 (ass, rhs);
  }
  
  /* Return the third operand on the RHS of assignment statement GS.
     If GS does not have two operands, NULL is returned instead.  */
  
  static inline tree
! gimple_assign_rhs3 (const gassign *gs)
  {
    if (gimple_num_ops (gs) >= 4)
!     return gs->op[3];
    else
      return NULL_TREE;
  }
  
+ static inline tree
+ gimple_assign_rhs3 (const_gimple gs)
+ {
+   const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
+   return gimple_assign_rhs3 (ass);
+ }
+ 
  /* Return a pointer to the third operand on the RHS of assignment
     statement GS.  */
  
  static inline tree *
  gimple_assign_rhs3_ptr (const_gimple gs)
  {
!   const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
!   gcc_gimple_checking_assert (gimple_num_ops (gs) >= 4);
!   return const_cast<tree *> (&ass->op[3]);
  }
  
  
  /* Set RHS to be the third operand on the RHS of assignment statement GS.  */
  
  static inline void
! gimple_assign_set_rhs3 (gassign *gs, tree rhs)
  {
!   gcc_gimple_checking_assert (gimple_num_ops (gs) >= 4);
!   gs->op[3] = rhs;
! }
  
! static inline void
! gimple_assign_set_rhs3 (gimple gs, tree rhs)
! {
!   gassign *ass = GIMPLE_CHECK2<gassign *> (gs);
!   gimple_assign_set_rhs3 (ass, rhs);
  }
  
+ 
  /* A wrapper around 3 operand gimple_assign_set_rhs_with_ops, for callers
     which expect to see only two operands.  */
  
*************** gimple_assign_set_nontemporal_move (gimp
*** 2499,2519 ****
     tree code of the object.  */
  
  static inline enum tree_code
! gimple_assign_rhs_code (const_gimple gs)
  {
!   enum tree_code code;
!   GIMPLE_CHECK (gs, GIMPLE_ASSIGN);
! 
!   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));
  
    return code;
  }
  
  
  /* Set CODE to be the code for the expression computed on the RHS of
     assignment S.  */
--- 2628,2652 ----
     tree code of the object.  */
  
  static inline enum tree_code
! gimple_assign_rhs_code (const gassign *gs)
  {
!   enum tree_code 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 (gs->op[1]);
  
    return code;
  }
  
+ static inline enum tree_code
+ gimple_assign_rhs_code (const_gimple gs)
+ {
+   const gassign *ass = GIMPLE_CHECK2<const gassign *> (gs);
+   return gimple_assign_rhs_code (ass);
+ }
+ 
  
  /* Set CODE to be the code for the expression computed on the RHS of
     assignment S.  */
Index: gcc/gimple.c
===================================================================
*** gcc/gimple.c	(revision 226490)
--- gcc/gimple.c	(working copy)
*************** static const char * const gimple_alloc_k
*** 89,94 ****
--- 89,98 ----
      "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.  */



More information about the Gcc-patches mailing list