public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][RFC] Re-work GIMPLE checking to be gimple class friendly
@ 2015-07-27 10:55 Richard Biener
  2015-07-27 11:34 ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2015-07-27 10:55 UTC (permalink / raw)
  To: gcc-patches


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.

I think inliner-wise we are even as we get rid of the gimple_op ()
inline but instead get the GIMPLE_CHECK2 inline.

So - any comments?

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_rhs1): Use GIMPLE_CHECK2 and a cheaper way
	to access operand 1.
	* gimple.c (gassign::code_): Define.

Index: gcc/gimple.h
===================================================================
--- gcc/gimple.h	(revision 226240)
+++ gcc/gimple.h	(working copy)
@@ -51,6 +51,16 @@ 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, const char *file = __builtin_FILE (),
+	      int line = __builtin_LINE (), const char *fun = __builtin_FUNCTION ())
+{
+  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
@@ -832,6 +842,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 +875,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;
@@ -2359,8 +2378,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];
 }
 
 
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.  */

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-08-12  7:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-27 10:55 [PATCH][RFC] Re-work GIMPLE checking to be gimple class friendly Richard Biener
2015-07-27 11:34 ` Richard Biener
2015-07-27 12:44   ` Richard Biener
2015-07-27 15:22     ` Michael Matz
2015-07-27 15:57       ` Richard Biener
2015-08-03 13:39     ` [PATCH][RFC] Re-work GIMPLE checking to be gimple class friendly (even better) Richard Biener
2015-08-12  7:42       ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).