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 <rguent...@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. */