On Wed, 13 Mar 2013, Xinliang David Li wrote:

> 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.

Note that we already have a perfectly working "short" interface for this.

  t = force_gimple_operand_gsi (... fold_build (....))

where you can in one place construct a recursive tree.

Richard.

> 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 */
> 
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend

Reply via email to