From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 57966 invoked by alias); 12 Aug 2016 07:13:49 -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 57950 invoked by uid 89); 12 Aug 2016 07:13:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=applicable X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 12 Aug 2016 07:13:47 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 4B7F7ACFD; Fri, 12 Aug 2016 07:13:45 +0000 (UTC) Date: Fri, 12 Aug 2016 07:13:00 -0000 From: Richard Biener To: Bernd Edlinger cc: Eric Botcazou , "gcc-patches@gcc.gnu.org" , Jeff Law , Jakub Jelinek Subject: Re: [PATCH] Increase alignment for bit-field access when predictive commoning (PR 71083) In-Reply-To: Message-ID: References: <7783559.v48Yu3xPbD@arcturus.home> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2016-08/txt/msg00992.txt.bz2 On Thu, 11 Aug 2016, Bernd Edlinger wrote: > On 08/11/16 09:07, Richard Biener wrote: > > The patch looks mostly ok, but > > > > + else > > + { > > + boff >>= LOG2_BITS_PER_UNIT; > > + boff += tree_to_uhwi (component_ref_field_offset (ref)); > > + coff = size_binop (MINUS_EXPR, coff, ssize_int (boff)); > > > > how can we be sure that component_ref_field_offset is an INTEGER_CST? > > At least Ada can have variably-length fields before a bitfield. We'd > > need to apply component_ref_field_offset to off in that case. Which > > makes me wonder if we can simply avoid the COMPONENT_REF path in > > a first iteration of the patch and always build a BIT_FIELD_REF? > > It should solve the correctness issues as well and be more applicable > > for branches. > > > > I believe that it will be necessary to check for tree_fits_uhwi_p here, > but while it happens quite often hat a normal COMPONENT_REF has a > variable offset, it happens rarely that a bit-field COMPONENT_REF has a > variable offset: In the whole Ada test suite it was only on the pack16 > test case. However I was not able to use that trick in the > loop_optimization23 test case. When I tried, it did no longer get > optimized by pcom. So there will be no new test case at this time. > > Therefore I left the comment as-is, because it is not clear, if a > variable offset will ever happen here; but if it happens, it will be > in Ada, and it will be safe to create a BIT_FIELD_REF instead. > > So this is the follow-up patch that tries to create a more aligned > access using the COMPONENT_REF. > > > Boot-strap and reg-test on x86_64-pc-linux-gnu. > Is it OK for trunk? Ok. Thanks, Richard.