On Mon, 3 Aug 2015, Richard Biener wrote:
> 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.
Applied to trunk as r226802.
Richard.
> Richard.
>
> 2015-07-27 Richard Biener <rguent...@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. */
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham
Norton, HRB 21284 (AG Nuernberg)
