From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 775 invoked by alias); 6 Jan 2007 13:31:38 -0000 Received: (qmail 766 invoked by uid 22791); 6 Jan 2007 13:31:38 -0000 X-Spam-Check-By: sourceware.org Received: from nf-out-0910.google.com (HELO nf-out-0910.google.com) (64.233.182.190) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 06 Jan 2007 13:31:34 +0000 Received: by nf-out-0910.google.com with SMTP id a25so448521nfc for ; Sat, 06 Jan 2007 05:31:32 -0800 (PST) Received: by 10.82.172.15 with SMTP id u15mr2821303bue.1168090292050; Sat, 06 Jan 2007 05:31:32 -0800 (PST) Received: by 10.82.150.13 with HTTP; Sat, 6 Jan 2007 05:31:32 -0800 (PST) Message-ID: <84fc9c000701060531g4b2da007mf5434b3fce63ec24@mail.gmail.com> Date: Sat, 06 Jan 2007 13:31:00 -0000 From: "Richard Guenther" To: "Eric Botcazou" Subject: Re: [PATCH] Tree SRA and atomicity/volatility Cc: gcc-patches@gcc.gnu.org In-Reply-To: <200701061422.39157.ebotcazou@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <200701061422.39157.ebotcazou@adacore.com> 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: 2007-01/txt/msg00459.txt.bz2 On 1/6/07, Eric Botcazou wrote: > Hi, > > The Ada language has the concept of atomicity, for example: > > type Word is > record > First, Second, Third, Fourth : Byte; > end record; > > External : Word := (others => 0); > pragma Atomic (External); > > Quoting the Ada RM: "For an atomic object (including an atomic component) all > reads and updates of the object as a whole are indivisible." > > The GNAT compiler implements it by (among other things) declaring the object > as being volatile. While this works well in 3.x, this breaks in 4.x because > the SRA may decompose accesses to volatile aggregates: > > eric@linux:~/build/gcc/native> cat t.c > struct S > { > char a; > char b; > char c; > char d; > }; > > void foo(void) > { > volatile struct S dev; > struct S tmp = dev; > tmp.a = 0; > dev = tmp; > } > > In .sra at -O: > > ;; Function foo (foo) > > Cannot scalarize variable dev because it is declared volatile > > [...] > > foo () > { > char tmp$d; > char tmp$c; > char tmp$b; > char tmp$a; > struct S tmp; > struct S dev; > > : > tmp$d_6 = dev.d; > tmp$c_7 = dev.c; > tmp$b_8 = dev.b; > tmp$a_9 = dev.a; > tmp$a_10 = 0; > dev.d = tmp$d_6; > dev.c = tmp$c_7; > dev.b = tmp$b_8; > dev.a = tmp$a_10; > return; > > } It doesn't scalarize dev - it scalarizes tmp, which is not volatile. Then it uses by-element copy, which is maybe not a good idea here (but not necessarily wrong). The problem is really that the middle-end doesn't have a good definition of volatile semantics on a structure. At least not what you want from it. It looks to me that you want either struct dev { volatile unsigned int data; }; and "decompose" data explicitly. What is the required semantics if I only access dev.a? Do you require a full-word load of the data and an extraction of a there? > but accesses to the volatile object are nevertheless decomposed. On x86-64: > > foo: > .LFB2: > movzbl -21(%rsp), %eax > movzbl -22(%rsp), %edx > movzbl -23(%rsp), %ecx > movzbl -24(%rsp), %esi > movb %al, -21(%rsp) > movb %dl, -22(%rsp) > movb %cl, -23(%rsp) > movb $0, -24(%rsp) > ret > > > While this may or may not be a problem for the C language, I think that it's a > real bug in the Tree SRA pass, which doesn't check whether the object to be > decomposed has side-effects. More specifically, this is the LDST case: > > /* Invoked when we have a copy between one scalarizable reference ELT > and one non-scalarizable reference OTHER. IS_OUTPUT is true if ELT > is on the left-hand side. */ > void (*ldst) (struct sra_elt *elt, tree other, > block_stmt_iterator *bsi, bool is_output); > > /* Scalarize a LDST. To recap, this is an assignment between one scalarizable > reference ELT and one non-scalarizable reference OTHER. IS_OUTPUT is true > if ELT is on the left-hand side. */ > > static void > scalarize_ldst (struct sra_elt *elt, tree other, > block_stmt_iterator *bsi, bool is_output) > { > /* Shouldn't have gotten called for a scalar. */ > gcc_assert (!elt->replacement); > > if (elt->use_block_copy) > { > /* Since ELT is not fully instantiated, we have to leave the > block copy in place. Treat this as a USE. */ > scalarize_use (elt, NULL, bsi, is_output, false); > } > else > { > /* The interesting case is when ELT is fully instantiated. In this > case we can have each element stored/loaded directly to/from the > corresponding slot in OTHER. This avoids a block copy. */ > > The last comment is wrong if OTHER has side-effects. Hence the attached > patch, tested on x86_64-suse-linux, which yields on x86-64: > > foo: > .LFB2: > movl -24(%rsp), %eax > movb $0, %al > movl %eax, -24(%rsp) > ret > > > This is a regression from the 3.x series for the Ada compiler so I'm > requesting approval for mainline/4.2/4.1 branches. No. This is not the correct solution for your problem. > > 2007-01-06 Eric Botcazou > > * tree-sra.c (sra_walk_fns) : Document new restriction. > (sra_walk_modify_expr) : Treat the reference as a use > if the lhs has side-effects. > : Treat the reference as a use if the rhs has side-effects. > > > > :ADDPATCH Tree-SRA: > > -- > Eric Botcazou > > >