From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8715 invoked by alias); 25 May 2011 10:06:37 -0000 Received: (qmail 8705 invoked by uid 22791); 25 May 2011 10:06:36 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from ksp.mff.cuni.cz (HELO atrey.karlin.mff.cuni.cz) (195.113.26.206) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 25 May 2011 10:06:22 +0000 Received: by atrey.karlin.mff.cuni.cz (Postfix, from userid 4018) id 49DCBF044C; Wed, 25 May 2011 12:06:21 +0200 (CEST) Date: Wed, 25 May 2011 12:18:00 -0000 From: Jan Hubicka To: Richard Guenther Cc: Jan Hubicka , gcc-patches@gcc.gnu.org, rguenther@suse.de Subject: Re: Faster streaming of enums Message-ID: <20110525100621.GA4810@atrey.karlin.mff.cuni.cz> References: <20110525094536.GE17175@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-2 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.18 (2008-05-17) X-IsSubscribed: yes 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: 2011-05/txt/msg01825.txt.bz2 > > *************** lto_output_tree (struct output_block *ob > > *** 1401,1407 **** > >         will instantiate two different nodes for the same object.  */ > >        output_record_start (ob, LTO_tree_pickle_reference); > >        output_uleb128 (ob, ix); > > !       output_uleb128 (ob, lto_tree_code_to_tag (TREE_CODE (expr))); > >      } > >    else if (lto_stream_as_builtin_p (expr)) > >      { > > --- 1399,1405 ---- > >         will instantiate two different nodes for the same object.  */ > >        output_record_start (ob, LTO_tree_pickle_reference); > >        output_uleb128 (ob, ix); > > !       output_record_start (ob, lto_tree_code_to_tag (TREE_CODE (expr))); > > I'd prefer lto_output_enum here as we don't really start a new output > record but just emit something for a sanity check. OK, I wondered what is cleaner, will update the patch. > > + /* Output VAL into OBS and verify it is in range MIN...MAX that is supposed > > +    to be compile time constant. > > +    Be host independent, limit range to 31bits.  */ > > + > > + static inline void > > + lto_output_int_in_range (struct lto_output_stream *obs, > > +                        HOST_WIDE_INT min, > > +                        HOST_WIDE_INT max, > > +                        HOST_WIDE_INT val) > > + { > > +   HOST_WIDE_INT range = max - min; > > + > > +   gcc_checking_assert (val >= min && val <= max && range > 0 > > +                      && range < 0x7fffffff); > > + > > +   val -= min; > > +   lto_output_1_stream (obs, val & 255); > > +   if (range >= 0xff) > > +     lto_output_1_stream (obs, (val << 8) & 255); > > +   if (range >= 0xffff) > > +     lto_output_1_stream (obs, (val << 16) & 255); > > +   if (range >= 0xffffff) > > +     lto_output_1_stream (obs, (val << 24) & 255); > > so you didn't want to create a bitpack_pack_int_in_range and use > bitpacks for enums? I suppose for some of the remaining cases > packing them into existing bitpacks would be preferable? Well, in my TODO list is to have both. Where we don't bitpatck enums with other values (that is the most common case of enums) this way we produce less overhead and have extra sanity check that the bits unused by enum are really 0. I guess final API should have both lto_output_enum and lto_bitpack_output_enum. I don't really care if the first have the implementation above or just creates its own bitpack to handle the value. > > + { > > +   HOST_WIDE_INT range = max - min; > > +   HOST_WIDE_INT val = lto_input_1_unsigned (ib); > > + > > +   gcc_checking_assert (range > 0); > > The assert doesn't match the one during output. Hmm, OK, will match. Honza