From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15718 invoked by alias); 21 Jun 2010 11:57:43 -0000 Received: (qmail 15704 invoked by uid 22791); 21 Jun 2010 11:57:42 -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 mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 21 Jun 2010 11:57:37 +0000 Received: (qmail 447 invoked from network); 21 Jun 2010 11:57:34 -0000 Received: from unknown (HELO ?192.168.44.101?) (nathan@127.0.0.2) by mail.codesourcery.com with ESMTPA; 21 Jun 2010 11:57:34 -0000 Message-ID: <4C1F5380.1090107@codesourcery.com> Date: Mon, 21 Jun 2010 12:44:00 -0000 From: Nathan Sidwell User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.9) Gecko/20100423 Thunderbird/3.0.4 MIME-Version: 1.0 To: GCC Patches Subject: [gimple] assignments to volatile Content-Type: multipart/mixed; boundary="------------020200000902030803020103" 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: 2010-06/txt/msg02007.txt.bz2 This is a multi-part message in MIME format. --------------020200000902030803020103 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1320 I've come across inconsistent and surprising behaviour when assigning to volatile lvalues. Sometimes we re-read the assigned-to object, and sometimes we do not. For instance, return vobj = data; will cause a reread of vobj, IF data is not a constant. or cond ? vobj = data : 0 will cause a reread of vobj, even when we don't use the result of the conditional expression -- unless data is a constant, in which case there is no-reread. (And confusingly, if you put void casts inside the two conditional sub-expressions, the re-reading goes away, but it doesn't even when you explicitly cast the whole expression to void). In the attached testcase, test_1, test_6 and test_7 have this surprising rereading behaviour. The fault appears to be in gimplify_modify_expr, where we return the LHS as the value, if the caller wants the value. The attached patch changes that routine when the target lvalue is volatile. In that case it will create a temporary to hold the RHS value, assign that temporary to the LHS and then return the temporary. Although the bug is architecture-neutral, the testcase is x86-specific because it's checking for specific accesses occurring. built and tested on i686-pc-linux-gnu, ok? nathan -- Nathan Sidwell :: http://www.codesourcery.com :: CodeSourcery --------------020200000902030803020103 Content-Type: text/x-patch; name="volatile.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="volatile.patch" Content-length: 4198 2010-06-21 Nathan Sidwell gcc/ * gimplify.c (gimplify_modify_expr): When assigning to volatiles, copy the src value and return a copy. gcc/testsuite/ * gcc.target/i386/volatile-2.c: New. Index: gimplify.c =================================================================== --- gimplify.c (revision 160876) +++ gimplify.c (working copy) @@ -4467,6 +4467,28 @@ gcc_assert (TREE_CODE (*expr_p) == MODIFY_EXPR || TREE_CODE (*expr_p) == INIT_EXPR); + if (want_value && TREE_THIS_VOLATILE (TREE_TYPE (*to_p))) + { + /* copy the rhs to a temporary, store the temporary and then + return it. This makes sure we consistently do not re-read a + volatile just after storing it. */ + tree lhs = *to_p; + tree rhs = *from_p; + tree nv_type = TYPE_MAIN_VARIANT (TREE_TYPE (lhs)); + tree tmp = create_tmp_var (nv_type, "modtmp"); + tree tmp_ass = build2 (MODIFY_EXPR, nv_type, tmp, rhs); + tree vol_ass = build2 (MODIFY_EXPR, nv_type, lhs, tmp); + + recalculate_side_effects (tmp_ass); + gimplify_and_add (tmp_ass, pre_p); + + recalculate_side_effects (vol_ass); + gimplify_and_add (vol_ass, pre_p); + + *expr_p = tmp; + return GS_OK; + } + /* Insert pointer conversions required by the middle-end that are not required by the frontend. This fixes middle-end type checking for for example gcc.dg/redecl-6.c. */ Index: testsuite/gcc.target/i386/volatile-2.c =================================================================== --- testsuite/gcc.target/i386/volatile-2.c (revision 0) +++ testsuite/gcc.target/i386/volatile-2.c (revision 0) @@ -0,0 +1,92 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +/* Check volatiles are written, read or not re-read consistently */ + + +/* simple assignments */ + +extern int volatile obj_0; +void test_0 (int data) +{ + /* should not reread obj */ + /* { dg-final { scan-assembler "movl\[ \t\]\[^,\]+, obj_0" } } */ + /* { dg-final { scan-assembler-not "movl\[ \t\]obj_0," } } */ + obj_0 = data; +} + +extern int volatile obj_1; +int test_1 (int data) +{ + /* should not reread obj */ + /* { dg-final { scan-assembler "movl\[ \t\]\[^,\]+, obj_1" } } */ + /* { dg-final { scan-assembler-not "movl\[ \t\]obj_1," } } */ + return obj_1 = data; +} + +extern int volatile obj_2; +int test_2 (void) +{ + /* should not reread obj */ + /* { dg-final { scan-assembler "movl\[ \t\]\[^,\]+, obj_2" } } */ + /* { dg-final { scan-assembler-not "movl\[ \t\]obj_2," } } */ + return obj_2 = 0; +} + + +/* Assignments in compound exprs */ + +extern int volatile obj_3; +int test_3 (int data) +{ + /* should not reread obj */ + /* { dg-final { scan-assembler "movl\[ \t\]\[^,\]+, obj_3" } } */ + /* { dg-final { scan-assembler-not "movl\[ \t\]obj_3," } } */ + return (obj_3 = data, 0); +} + +extern int volatile obj_4; +int test_4 (void) +{ + /* should not reread obj */ + /* { dg-final { scan-assembler "movl\[ \t\]\[^,\]+, obj_4" } } */ + /* { dg-final { scan-assembler-not "movl\[ \t\]obj_4," } } */ + return (obj_4 = 0, 0); +} +extern int volatile obj_5; +int test_5 (void) +{ + /* should reread obj */ + /* { dg-final { scan-assembler "movl\[ \t\]\[^,\]+, obj_5" } } */ + /* { dg-final { scan-assembler "movl\[ \t\]obj_5," } } */ + return (obj_5 = 0, obj_5); +} + +/* Assignments in conditional exprs */ + +extern int volatile obj_6; +void test_6 (int data, int cond) +{ + /* should not reread obj */ + /* { dg-final { scan-assembler "movl\[ \t\]\[^,\]+, obj_6" } } */ + /* { dg-final { scan-assembler-not "movl\[ \t\]obj_6," } } */ + cond ? obj_6 = data : 0; +} + +extern int volatile obj_7; +int test_7 (int data, int cond) +{ + /* should not reread obj */ + /* { dg-final { scan-assembler "movl\[ \t\]\[^,\]+, obj_7" } } */ + /* { dg-final { scan-assembler-not "movl\[ \t\]obj_7," } } */ + return cond ? obj_7 = data : 0; +} + +extern int volatile obj_8; +int test_8 (int cond) +{ + /* should not reread obj */ + /* { dg-final { scan-assembler "movl\[ \t\]\[^,\]+, obj_8" } } */ + /* { dg-final { scan-assembler-not "movl\[ \t\]obj_8," } } */ + return cond ? obj_8 = 0 : 0; +} --------------020200000902030803020103--