Nice -- GCC LOC will be significantly reduced with these interfaces. Using 'add' as interface name can be confusing -- new, or new_stmt, or new_assignment/new_call etc might be better -- but we can delay the bike shedding later.
David On Wed, Mar 13, 2013 at 2:55 PM, Diego Novillo <dnovi...@google.com> wrote: > This patch adds an initial implementation for a new helper type for > generating GIMPLE statements. > > The type is called gimple_builder. There are two main variants: > gimple_builder_normal and gimple_builder_ssa. The difference between > the two is the temporaries they create. The 'normal' builder creates > temporaries in normal form (i.e., VAR_DECLs). The 'ssa' builder > creates SSA names. > > The basic functionality is described in > http://gcc.gnu.org/wiki/cxx-conversion/gimple-generation. I expect it > to evolve as I address feedback on this initial implementation. > > The patch implements the initial builder. It has enough functionality > to simplify the generation of 3 address assignments (the bulk of all > generated code). > > To use the builder: > > 1- Declare an instance 'gb' of gimple_builder_normal or > gimple_builder_ssa. E.g., gimple_builder_ssa gb; > > 2- Use gb.add_* to add a new statement to it. This > returns an SSA name or VAR_DECL with the value of the added > statement. > > 3- Call gb.insert_*() to insert the sequence of statements in the > builder into a statement iterator. > > For instance, in asan.c we generate the expression: > > (shadow != 0) & (base_addr & 7) + (size_in_bytes - 1)) >= shadow). > > with the following code: > > ----------------------------------------------------------------------------- > gimple_builder_ssa gb(location); > t = gb.add (NE_EXPR, shadow, 0); > tree t1 = gb.add (BIT_AND_EXPR, base_addr, 7); > t1 = gb.add_type_cast (shadow_type, t1); > if (size_in_bytes > 1) > t1 = gb.add (PLUS_EXPR, t1, size_in_bytes - 1); > t1 = gb.add (GE_EXPR, t1, shadow); > t = gb.add (BIT_AND_EXPR, t, t1); > gb.insert_after (&gsi, GSI_NEW_STMT); > ----------------------------------------------------------------------------- > > > In contrast, the original code needed to generate the same expression > is significantly longer: > > > ----------------------------------------------------------------------------- > g = gimple_build_assign_with_ops (NE_EXPR, > make_ssa_name (boolean_type_node, > NULL), > shadow, > build_int_cst (shadow_type, 0)); > gimple_set_location (g, location); > gsi_insert_after (&gsi, g, GSI_NEW_STMT); > t = gimple_assign_lhs (g); > > g = gimple_build_assign_with_ops (BIT_AND_EXPR, > make_ssa_name (uintptr_type, > NULL), > base_addr, > build_int_cst (uintptr_type, 7)); > gimple_set_location (g, location); > gsi_insert_after (&gsi, g, GSI_NEW_STMT); > > g = gimple_build_assign_with_ops (NOP_EXPR, > make_ssa_name (shadow_type, > NULL), > gimple_assign_lhs (g), NULL_TREE); > gimple_set_location (g, location); > gsi_insert_after (&gsi, g, GSI_NEW_STMT); > if (size_in_bytes > 1) > { > g = gimple_build_assign_with_ops (PLUS_EXPR, > make_ssa_name (shadow_type, > NULL), > gimple_assign_lhs (g), > build_int_cst (shadow_type, > size_in_bytes - > 1)); > gimple_set_location (g, location); > gsi_insert_after (&gsi, g, GSI_NEW_STMT); > } > > g = gimple_build_assign_with_ops (GE_EXPR, > make_ssa_name (boolean_type_node, > NULL), > gimple_assign_lhs (g), > shadow); > gimple_set_location (g, location); > gsi_insert_after (&gsi, g, GSI_NEW_STMT); > > g = gimple_build_assign_with_ops (BIT_AND_EXPR, > make_ssa_name (boolean_type_node, > NULL), > t, gimple_assign_lhs (g)); > gimple_set_location (g, location); > gsi_insert_after (&gsi, g, GSI_NEW_STMT); > t = gimple_assign_lhs (g); > ----------------------------------------------------------------------------- > > I expect to add more facilities to the builder. Mainly, generation of > control flow altering statements which will automatically reflect on > the CFG. > > I do not think the helper should replace all code generation, but it > should serve as a shorter/simpler way of generating GIMPLE IL in the > common cases. > > Feedback welcome. I would like to consider adding this facility when > stage 1 opens. > > In the meantime, I've committed the patch to the cxx-conversion > branch. > > > Thanks. Diego. > > diff --git a/gcc/asan.c b/gcc/asan.c > index af9c01e..571882a 100644 > --- a/gcc/asan.c > +++ b/gcc/asan.c > @@ -1379,57 +1379,15 @@ build_check_stmt (location_t location, tree base, > gimple_stmt_iterator *iter, > /* Slow path for 1, 2 and 4 byte accesses. > Test (shadow != 0) > & ((base_addr & 7) + (size_in_bytes - 1)) >= shadow). */ > - g = gimple_build_assign_with_ops (NE_EXPR, > - make_ssa_name (boolean_type_node, > - NULL), > - shadow, > - build_int_cst (shadow_type, 0)); > - gimple_set_location (g, location); > - gsi_insert_after (&gsi, g, GSI_NEW_STMT); > - t = gimple_assign_lhs (g); > - > - g = gimple_build_assign_with_ops (BIT_AND_EXPR, > - make_ssa_name (uintptr_type, > - NULL), > - base_addr, > - build_int_cst (uintptr_type, 7)); > - gimple_set_location (g, location); > - gsi_insert_after (&gsi, g, GSI_NEW_STMT); > - > - g = gimple_build_assign_with_ops (NOP_EXPR, > - make_ssa_name (shadow_type, > - NULL), > - gimple_assign_lhs (g), NULL_TREE); > - gimple_set_location (g, location); > - gsi_insert_after (&gsi, g, GSI_NEW_STMT); > - > + gimple_builder_ssa gb(location); > + t = gb.add (NE_EXPR, shadow, 0); > + tree t1 = gb.add (BIT_AND_EXPR, base_addr, 7); > + t1 = gb.add_type_cast (shadow_type, t1); > if (size_in_bytes > 1) > - { > - g = gimple_build_assign_with_ops (PLUS_EXPR, > - make_ssa_name (shadow_type, > - NULL), > - gimple_assign_lhs (g), > - build_int_cst (shadow_type, > - size_in_bytes - > 1)); > - gimple_set_location (g, location); > - gsi_insert_after (&gsi, g, GSI_NEW_STMT); > - } > - > - g = gimple_build_assign_with_ops (GE_EXPR, > - make_ssa_name (boolean_type_node, > - NULL), > - gimple_assign_lhs (g), > - shadow); > - gimple_set_location (g, location); > - gsi_insert_after (&gsi, g, GSI_NEW_STMT); > - > - g = gimple_build_assign_with_ops (BIT_AND_EXPR, > - make_ssa_name (boolean_type_node, > - NULL), > - t, gimple_assign_lhs (g)); > - gimple_set_location (g, location); > - gsi_insert_after (&gsi, g, GSI_NEW_STMT); > - t = gimple_assign_lhs (g); > + t1 = gb.add (PLUS_EXPR, t1, size_in_bytes - 1); > + t1 = gb.add (GE_EXPR, t1, shadow); > + t = gb.add (BIT_AND_EXPR, t, t1); > + gb.insert_after (&gsi, GSI_NEW_STMT); > } > else > t = shadow; > diff --git a/gcc/gimple.c b/gcc/gimple.c > index 785c2f0..c4687df 100644 > --- a/gcc/gimple.c > +++ b/gcc/gimple.c > @@ -4210,4 +4210,115 @@ gimple_asm_clobbers_memory_p (const_gimple stmt) > > return false; > } > + > + > +/* Return the expression type to use based on the CODE and type of > + the given operand OP. If the expression CODE is a comparison, > + the returned type is boolean_type_node. Otherwise, it returns > + the type of OP. */ > + > +tree > +gimple_builder_base::get_expr_type (enum tree_code code, tree op) > +{ > + return (TREE_CODE_CLASS (code) == tcc_comparison) > + ? boolean_type_node > + : TREE_TYPE (op); > +} > + > + > +/* Add a new assignment to this GIMPLE sequence. The assignment has > + the form: GIMPLE_ASSIGN <CODE, LHS, OP1, OP2>. Returns LHS. */ > + > +tree > +gimple_builder_base::add (enum tree_code code, tree lhs, tree op1, tree op2) > +{ > + gimple s = gimple_build_assign_with_ops (code, lhs, op1, op2); > + gimple_seq_add_stmt (&seq_, s); > + return lhs; > +} > + > + > +/* Add a new assignment to this GIMPLE sequence. The new assignment will > + have the opcode CODE and operands OP1 and OP2. The type of the > + expression on the RHS is inferred to be the type of OP1. > + > + The LHS of the statement will be an SSA name or a GIMPLE temporary > + in normal form depending on the type of builder invoking this > + function. */ > + > +tree > +gimple_builder_base::add (enum tree_code code, tree op1, tree op2) > +{ > + tree lhs = create_lhs_for_assignment (get_expr_type (code, op1)); > + return add (code, lhs, op1, op2); > +} > + > + > +/* Add a new assignment to this GIMPLE sequence. The new > + assignment will have the opcode CODE and operands OP1 and VAL. > + VAL is converted into a an INTEGER_CST of the same type as OP1. > + > + The LHS of the statement will be an SSA name or a GIMPLE temporary > + in normal form depending on the type of builder invoking this > + function. */ > + > +tree > +gimple_builder_base::add (enum tree_code code, tree op1, int val) > +{ > + tree type = get_expr_type (code, op1); > + tree op2 = build_int_cst (TREE_TYPE (op1), val); > + tree lhs = create_lhs_for_assignment (type); > + return add (code, lhs, op1, op2); > +} > + > + > +/* Add a type cast assignment to this GIMPLE sequence. This creates a > NOP_EXPR > + that converts OP to TO_TYPE. Return the LHS of the generated assignment. > */ > + > +tree > +gimple_builder_base::add_type_cast (tree to_type, tree op) > +{ > + tree lhs = create_lhs_for_assignment (to_type); > + return add (NOP_EXPR, lhs, op, NULL_TREE); > +} > + > + > +/* Insert this sequence after the statement pointed-to by iterator I. > + MODE is an is gs_insert_after. Scan the statements in SEQ for new > + operands. */ > + > +void > +gimple_builder_base::insert_after (gimple_stmt_iterator *i, > + enum gsi_iterator_update mode) > +{ > + /* Since we are inserting a sequence, the semantics for GSI_NEW_STMT > + are not quite what the caller is expecting. GSI_NEW_STMT will > + leave the iterator pointing to the *first* statement of this > + sequence. Rather, we want the iterator to point to the *last* > + statement in the sequence. Therefore, we use > + GSI_CONTINUE_LINKING when GSI_NEW_STMT is requested. */ > + if (mode == GSI_NEW_STMT) > + mode = GSI_CONTINUE_LINKING; > + gsi_insert_seq_after (i, seq_, mode); > +} > + > + > +/* Create a GIMPLE temporary type TYPE to be used as the LHS of an > + assignment. */ > + > +tree > +gimple_builder_normal::create_lhs_for_assignment (tree type) > +{ > + return create_tmp_var (type, NULL); > +} > + > + > +/* Create an SSA name of type TYPE to be used as the LHS of an assignment. > */ > + > +tree > +gimple_builder_ssa::create_lhs_for_assignment (tree type) > +{ > + return make_ssa_name (type, NULL); > +} > + > #include "gt-gimple.h" > diff --git a/gcc/gimple.h b/gcc/gimple.h > index 204c3c9..7b5e741 100644 > --- a/gcc/gimple.h > +++ b/gcc/gimple.h > @@ -5393,4 +5393,57 @@ extern tree maybe_fold_or_comparisons (enum tree_code, > tree, tree, > enum tree_code, tree, tree); > > bool gimple_val_nonnegative_real_p (tree); > + > + > +/* GIMPLE builder class. This type provides a simplified interface > + for generating new GIMPLE statements. */ > + > +class gimple_builder_base > +{ > +public: > + tree add (enum tree_code, tree, tree); > + tree add (enum tree_code, tree, int); > + tree add (enum tree_code, tree, tree, tree); > + tree add_type_cast (tree, tree); > + void insert_after (gimple_stmt_iterator *, enum gsi_iterator_update); > + > +protected: > + gimple_builder_base() : seq_(NULL), loc_(UNKNOWN_LOCATION) {} > + gimple_builder_base(location_t l) : seq_(NULL), loc_(l) {} > + tree get_expr_type (enum tree_code code, tree op); > + virtual tree create_lhs_for_assignment (tree) = 0; > + > +private: > + gimple_seq seq_; > + location_t loc_; > +}; > + > + > +/* GIMPLE builder class for statements in normal form. Statements generated > + by instances of this class will produce non-SSA temporaries. */ > + > +class gimple_builder_normal : public gimple_builder_base > +{ > +public: > + gimple_builder_normal() : gimple_builder_base() {} > + gimple_builder_normal(location_t l) : gimple_builder_base(l) {} > + > +protected: > + virtual tree create_lhs_for_assignment (tree); > +}; > + > + > +/* GIMPLE builder class for statements in normal form. Statements generated > + by instances of this class will produce SSA names. */ > + > +class gimple_builder_ssa : public gimple_builder_base > +{ > +public: > + gimple_builder_ssa() : gimple_builder_base() {} > + gimple_builder_ssa(location_t l) : gimple_builder_base(l) {} > + > +protected: > + virtual tree create_lhs_for_assignment (tree); > +}; > + > #endif /* GCC_GIMPLE_H */
