From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19916 invoked by alias); 14 Mar 2013 09:00:54 -0000 Received: (qmail 19906 invoked by uid 22791); 14 Mar 2013 09:00:52 -0000 X-SWARE-Spam-Status: No, hits=-7.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,TW_CX,TW_TM X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 14 Mar 2013 09:00:42 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id A28D5A51F7; Thu, 14 Mar 2013 10:00:40 +0100 (CET) Date: Thu, 14 Mar 2013 09:00:00 -0000 From: Richard Biener To: Xinliang David Li Cc: Diego Novillo , gcc-patches@gcc.gnu.org, Lawrence Crowl Subject: Re: [cxx-conversion] RFC - Helper types for building GIMPLE In-Reply-To: Message-ID: References: <20130313215516.GA29289@google.com> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2013-03/txt/msg00492.txt.bz2 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 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 . 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 SUSE / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 GF: Jeff Hawn, Jennifer Guild, Felix Imend