From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7954 invoked by alias); 12 Mar 2014 21:46:00 -0000 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 Received: (qmail 7941 invoked by uid 89); 12 Mar 2014 21:46:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_05,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ea0-f181.google.com Received: from mail-ea0-f181.google.com (HELO mail-ea0-f181.google.com) (209.85.215.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 12 Mar 2014 21:45:58 +0000 Received: by mail-ea0-f181.google.com with SMTP id k10so118432eaj.12 for ; Wed, 12 Mar 2014 14:45:55 -0700 (PDT) X-Received: by 10.14.179.73 with SMTP id g49mr51946eem.116.1394660755449; Wed, 12 Mar 2014 14:45:55 -0700 (PDT) Received: from nbbrfq.cc.univie.ac.at (85-127-95-210.dynamic.xdsl-line.inode.at. [85.127.95.210]) by mx.google.com with ESMTPSA id 43sm635190eeh.13.2014.03.12.14.45.53 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 12 Mar 2014 14:45:54 -0700 (PDT) Date: Wed, 12 Mar 2014 21:51:00 -0000 From: Bernhard Reutner-Fischer To: Zoran Jovanovic Cc: Richard Biener , "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH] Add a new option "-ftree-bitfield-merge" (patch / doc inside) Message-ID: <20140312214551.GA26753@nbbrfq.cc.univie.ac.at> References: <386B40EC5E8DBF459FD11A754D868AD922E31112@BADAG02.ba.imgtec.org> <386B40EC5E8DBF459FD11A754D868AD922E333D4@BADAG02.ba.imgtec.org> <386B40EC5E8DBF459FD11A754D868AD922E34DA7@BADAG02.ba.imgtec.org> <386B40EC5E8DBF459FD11A754D868AD96DA5A28A@BADAG02.ba.imgtec.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <386B40EC5E8DBF459FD11A754D868AD96DA5A28A@BADAG02.ba.imgtec.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2014-03/txt/msg00604.txt.bz2 On Sun, Mar 09, 2014 at 08:35:43PM +0000, Zoran Jovanovic wrote: > Hello, > This is new patch version. > Approach suggested by Richard Biener with lowering bit-field accesses instead of modifying gimple trees is implemented. > New command line option "-fmerge-bitfields" is introduced. > > Tested - passed gcc regression tests. > > Changelog - > > gcc/ChangeLog: > 2014-03-09 Zoran Jovanovic (zoran.jovanovic@imgtec.com) > * common.opt (fmerge-bitfields): New option. > * doc/invoke.texi: Added reference to "-fmerge-bitfields". Present tense. > * tree-sra.c (lower_bitfields): New function. > Entry for (-fmerge-bitfields). > (bfaccess::hash): New function. > (bfaccess::equal): New function. > (bfaccess::remove): New function. > (bitfield_access_p): New function. > (lower_bitfield_read): New function. > (lower_bitfield_write): New function. > (bitfield_stmt_access_pair_htab_hash): New function. > (bitfield_stmt_access_pair_htab_eq): New function. > (create_and_insert_access): New function. > (get_bit_offset): New function. > (get_merged_bit_field_size): New function. > (add_stmt_access_pair): New function. > (cmp_access): New function. > * dwarf2out.c (simple_type_size_in_bits): moved to tree.c. Present tense. Capital 'M'ove > (field_byte_offset): declaration moved to tree.h, static removed. Capital 'D'eclaration. These are supposed to be sentences. By removing static you IMHO 'make extern'. > * testsuite/gcc.dg/tree-ssa/bitfldmrg1.c: New test. > * testsuite/gcc.dg/tree-ssa/bitfldmrg2.c: New test. > * tree-ssa-sccvn.c (expressions_equal_p): moved to tree.c. See above. > * tree-ssa-sccvn.h (expressions_equal_p): declaration moved to tree.h. Likewise. > * tree.c (expressions_equal_p): moved from tree-ssa-sccvn.c. See above. > (simple_type_size_in_bits): moved from dwarf2out.c. See above. > * tree.h (expressions_equal_p): declaration added. Ditto. > (field_byte_offset): declaration added. Ditto. > > Patch - > > diff --git a/gcc/common.opt b/gcc/common.opt > index 661516d..3331d03 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -2193,6 +2193,10 @@ ftree-sra > Common Report Var(flag_tree_sra) Optimization > Perform scalar replacement of aggregates > > +fmerge-bitfields > +Common Report Var(flag_tree_bitfield_merge) Init(0) Optimization Optimization but not enabled for any level. So, where would one generally want this enabled? CSiBE numbers? SPEC you-name-it improvements? size(1) improvements where? In GCC there is generally no interest in the size(1) added to the collection itself, so let me ask for size(1) and bloat(-o-meter) stats for gcc, cc1 and collect2, just for the sake of it? > +Merge loads and stores of consecutive bitfields > + > ftree-ter > Common Report Var(flag_tree_ter) Optimization > Replace temporary expressions in the SSA->normal pass > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 24bd76e..54bae56 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -411,7 +411,7 @@ Objective-C and Objective-C++ Dialects}. > -fsplit-ivs-in-unroller -fsplit-wide-types -fstack-protector @gol > -fstack-protector-all -fstack-protector-strong -fstrict-aliasing @gol > -fstrict-overflow -fthread-jumps -ftracer -ftree-bit-ccp @gol > --ftree-builtin-call-dce -ftree-ccp -ftree-ch @gol > +-fmerge-bitfields -ftree-builtin-call-dce -ftree-ccp -ftree-ch @gol > -ftree-coalesce-inline-vars -ftree-coalesce-vars -ftree-copy-prop @gol > -ftree-copyrename -ftree-dce -ftree-dominator-opts -ftree-dse @gol > -ftree-forwprop -ftree-fre -ftree-loop-if-convert @gol > @@ -7807,6 +7807,11 @@ pointer alignment information. > This pass only operates on local scalar variables and is enabled by default > at @option{-O} and higher. It requires that @option{-ftree-ccp} is enabled. > > +@item -fbitfield-merge you are talking about '-fmerge-bitfields' up until here (except for Subject. [Confusion starts here -- Subject: -ftree-bitfield-merge; sofar Intro -fmerge-bitfields and ChangeLog -fmerge-bitfields] > +@opindex fmerge-bitfields > +Combines several adjacent bit-field accesses that copy values > +from one memory location to another into one single bit-field access. > + > @item -ftree-ccp > @opindex ftree-ccp > Perform sparse conditional constant propagation (CCP) on trees. This > diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c > index 284d544..c6a19b2 100644 > --- a/gcc/tree-sra.c > +++ b/gcc/tree-sra.c > @@ -3462,10 +3462,608 @@ perform_intra_sra (void) > return ret; > } > > +/* Bitfield access and hashtable support commoning same base and > + representative. */ > + > +struct bfaccess > +{ > + bfaccess (tree r):ref (r), r_count (1), w_count (1), merged (false), > + modified (false), is_barrier (false), next (0), head_access (0) > + { > + } > + > + tree ref; > + unsigned r_count; /* Read counter. */ > + unsigned w_count; /* Write counter. */ > + > + /* hash_table support. */ > + typedef bfaccess value_type; > + typedef bfaccess compare_type; > + static inline hashval_t hash (const bfaccess *); I suspect there's a reason to not have the * be a const* ? > + static inline int equal (const bfaccess *, const bfaccess *); ..which holds true here as well. > + static inline void remove (bfaccess *); > + > + gimple load_stmt; /* Bit-field load statement. */ > + gimple store_stmt; /* Bit-field store statement. */ > + unsigned src_offset_words; /* Bit-field offset at src in words. */ > + unsigned src_bit_offset; /* Bit-field offset inside source word. */ > + unsigned src_bit_size; /* Size of bit-field in source word. */ > + unsigned dst_offset_words; /* Bit-field offset at dst in words. */ > + unsigned dst_bit_offset; /* Bit-field offset inside destination > + word. */ > + unsigned src_field_offset; /* Source field offset. */ > + unsigned dst_bit_size; /* Size of bit-field in destination word. */ > + tree src_addr; /* Address of source memory access. */ > + tree dst_addr; /* Address of destination memory access. */ > + bool merged; /* True if access is merged with another > + one. */ > + bool modified; /* True if bit-field size is modified. */ > + bool is_barrier; /* True if access is barrier (call or mem > + access). */ Back then the above usually were bitfields:1 themselves and for space considerations were moved to a place where alignment begged for or with cacheline friendliness in mind but i guess those times are past nowadays, yes? > + struct bfaccess *next; /* Access with which this one is merged. */ > + tree bitfield_representative; /* Bit field representative of original > + declaration. */ > + struct bfaccess *head_access; /* Head of access list where this one is > + merged. */ > +}; > +/* Return whether REF is a bitfield access the bit offset of the bitfield mhm. Maybe it's late here by now, but can you actually parse the sentence above? Is there an 'at' missing somewhere? "a bitfield access at" perhaps? Here I'll stop attempts to follow what you wrote, no offence. TIA && cheers,