From patchwork Wed Aug 4 11:53:07 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jie Zhang X-Patchwork-Id: 60846 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id ECCFB1007D1 for ; Wed, 4 Aug 2010 21:53:23 +1000 (EST) Received: (qmail 4352 invoked by alias); 4 Aug 2010 11:53:22 -0000 Received: (qmail 4337 invoked by uid 22791); 4 Aug 2010 11:53:21 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 04 Aug 2010 11:53:15 +0000 Received: (qmail 9064 invoked from network); 4 Aug 2010 11:53:12 -0000 Received: from unknown (HELO ?192.168.1.100?) (jie@127.0.0.2) by mail.codesourcery.com with ESMTPA; 4 Aug 2010 11:53:12 -0000 Message-ID: <4C5954A3.8090003@codesourcery.com> Date: Wed, 04 Aug 2010 19:53:07 +0800 From: Jie Zhang User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.7) Gecko/20100713 Lightning/1.0b2 Thunderbird/3.1.1 MIME-Version: 1.0 To: GCC Patches Subject: Re: [RFC] Don't completely scalarize a record if it contains bit-field (PR tree-optimization/45144) References: <4C52F946.6010404@codesourcery.com> <20100802130118.GC15391@virgil.arch.suse.de> In-Reply-To: <20100802130118.GC15391@virgil.arch.suse.de> X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On 08/02/2010 09:01 PM, Martin Jambor wrote: > Hi, > > On Sat, Jul 31, 2010 at 12:09:42AM +0800, Jie Zhang wrote: >> PR tree-optimization/45144 shows an issue that SRA causes. I used >> arm-none-eabi target as an example in PR tree-optimization/45144. >> But the same issue can also been seen on x86_64-linux-gnu target >> using the same test case in the PR. >> >> SRA completely scalarizes a small record. But when the record is >> used later as a whole, GCC has to make the record out of the scalar >> parts. When the record contains bit-fields, GCC generates ugly code >> to assemble the scalar parts into a record. >> >> Until the aggregates copy propagation is implemented, I think it >> would better to disable full scalarization for such records. The >> patch is attached. It's bootstrapped on x86_64-linux-gnu and >> regression tested. >> >> Is it OK for now? We can remove it after aggregates copy propagation >> is implemented. >> >> Will it be better to add bit-field check in >> type_consists_of_records_p instead of using a new function >> "type_contains_bit_field_p"? >> > > When I was implementing the total scalarization bit of SRA I thought > of disabling it for structures with bit-fields too. I did not really > examine the effects in any way but I never expected this to result in > nice code at places where we use SRA to do poor-man's copy > propagation. However, eventually I decided to keep the total > scalarization for these structures because doing so can save stack > space and it would be shame if adding one such field to a structure > would make us use the space again (in fact, total scalarization was > introduced as a fix to unnecessary stack-frame setup bugs like PR > 42585). But given your results with kernel and gcc, I don't object to > disabling it... people will scream if something slows down for them. > > On the other hand, if we decide to go this way, we need to do the > check at a different place, going over the whole type whenever looking > at an assignment is not necessary and is wasteful. The check would be > most appropriate as a part of type_consists_of_records_p where it > would be performed only once for each variable in question. > Thanks for your comment! How about this version? I moved the check into type_consists_of_records_p. Bootstrapped and regression tested on x86_64. Also checked by building linux kernel and made sure there were no regressions. Regards, PR tree-optimization/45144 * tree-sra.c (type_consists_of_records_p): Return false if the record contains bit-field. testsuite/ PR tree-optimization/45144 * gcc.dg/tree-ssa/pr45144.c: New test. Index: testsuite/gcc.dg/tree-ssa/pr45144.c =================================================================== --- testsuite/gcc.dg/tree-ssa/pr45144.c (revision 0) +++ testsuite/gcc.dg/tree-ssa/pr45144.c (revision 0) @@ -0,0 +1,46 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +void baz (unsigned); + +extern unsigned buf[]; + +struct A +{ + unsigned a1:10; + unsigned a2:3; + unsigned:19; +}; + +union TMP +{ + struct A a; + unsigned int b; +}; + +static unsigned +foo (struct A *p) +{ + union TMP t; + struct A x; + + x = *p; + t.a = x; + return t.b; +} + +void +bar (unsigned orig, unsigned *new) +{ + struct A a; + union TMP s; + + s.b = orig; + a = s.a; + if (a.a1) + baz (a.a2); + *new = foo (&a); +} + +/* { dg-final { scan-tree-dump "x = a;" "optimized"} } */ +/* { dg-final { cleanup-tree-dump "optimized" } } */ Index: tree-sra.c =================================================================== --- tree-sra.c (revision 162725) +++ tree-sra.c (working copy) @@ -811,7 +811,7 @@ create_access (tree expr, gimple stmt, b /* Return true iff TYPE is a RECORD_TYPE with fields that are either of gimple register types or (recursively) records with only these two kinds of fields. It also returns false if any of these records has a zero-size field as its - last field. */ + last field or has a bit-field. */ static bool type_consists_of_records_p (tree type) @@ -827,6 +827,9 @@ type_consists_of_records_p (tree type) { tree ft = TREE_TYPE (fld); + if (DECL_BIT_FIELD (fld)) + return false; + if (!is_gimple_reg_type (ft) && !type_consists_of_records_p (ft)) return false;