From patchwork Wed Mar 21 17:35:32 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 148052 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 2507AB6FA8 for ; Thu, 22 Mar 2012 05:27:00 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1332959222; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Message-ID:Date:From:User-Agent:MIME-Version:To:Subject: Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:Sender:Delivered-To; bh=MTzmMf5 w+lbRdcWMug1kF+vcoa8=; b=OQ9Ed4UZ6cD+TZa4loJLbX4K8O/CGRwIkLAIwLJ wKPIdUKb95MuteoFYtRzfgFgCkqVRJM7zOg5TX6NyFF+ITb/YAmdR743i9yO5K+o PgwfgsDa3Ogv+P1PvrYv4UWhv/tawGyVscnp3NajrK/Ha82Rw/O/vQDHNcHrH+2e XlWY= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=o3a9I5qiGc6uKoLieGWMVF3WlmfE5MGFdHJgN4WpgYI7XTA7YJMfGqUiM0o6th G+HBm/YJ8BNKCRqWxm4egvnL62AjU9gw9W2WExAWA02bYOHjKFKJYlrhvPgMBRR6 vPl7ZfbPaVMWD8+E1J4zkTywR/oK+GM7E+K203VgGwtxo=; Received: (qmail 32281 invoked by alias); 21 Mar 2012 18:26:53 -0000 Received: (qmail 32264 invoked by uid 22791); 21 Mar 2012 18:26:48 -0000 X-SWARE-Spam-Status: No, hits=-6.7 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 21 Mar 2012 18:26:34 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q2LIQWlA014691 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 21 Mar 2012 14:26:34 -0400 Received: from houston.quesejoda.com (vpn-224-22.phx2.redhat.com [10.3.224.22]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q2LHZW77026383; Wed, 21 Mar 2012 13:35:33 -0400 Message-ID: <4F6A1164.7050207@redhat.com> Date: Wed, 21 Mar 2012 12:35:32 -0500 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.1) Gecko/20120216 Thunderbird/10.0.1 MIME-Version: 1.0 To: Andrew MacLeod , Torvald Riegel , gcc-patches Subject: [C11-atomic] test invalid hoist across and acquire load 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 In the test below, we cannot cache either [x] or [y] neither before the load of flag1 nor the load of flag2. This is because the corresponding store/release can flush a different value of x or y: + if (__atomic_load_n (&flag1, __ATOMIC_ACQUIRE)) + i = x + y; + + if (__atomic_load_n (&flag2, __ATOMIC_ACQUIRE)) + a = 10; + j = x + y; For example, on x86-64, we are hoisting "x" and "y" before the load of flag2: movl flag1(%rip), %eax movl x(%rip), %edx <-- hoist of X testl %eax, %eax movl y(%rip), %eax <-- hoist of Y je .L2 leal (%edx,%eax), %ecx movl %ecx, i(%rip) .L2: movl flag2(%rip), %ecx testl %ecx, %ecx je .L3 movl $10, a(%rip) .L3: addl %edx, %eax <-- x/y may have changed by the acquire of flag2. movl %eax, j(%rip) ret (For that matter, we are also hoisting "x" before the actual test of flag1 as well, but I believe this is allowed since flag1 has already been loaded.) The pass at fault here is the combine stack adjustment RTL pass. I have not looked into why this is happening, but I wanted to get this test into the branch lest we forget about it. Is this OK for the branch? Is my understanding correct? Aldy Index: atomic-hoist-1.c =================================================================== --- atomic-hoist-1.c (revision 0) +++ atomic-hoist-1.c (revision 0) @@ -0,0 +1,96 @@ +/* { dg-do link } */ +/* { dg-require-effective-target sync_int_long } */ +/* { dg-final { simulate-thread } } */ + +/* Test that a hoist is not performed across an acquire barrier. */ + +#include +#include "simulate-thread.h" + +int flag1=1, flag2=1; + +unsigned int x=1, y=2, i=0x1234, j=0x5678, a; + +/* These two tables are random numbers such that there are no two + pairs between the both tables that yield the same sum. */ + +unsigned int table1[16] = { + 24747, 19512, 3692, 25985, + 25079, 24, 3310, 22073, + 4026, 25641, 35240, 35542, + 24783, 17378, 12184, 23755 +}; + +unsigned int table2[16] = { + 2467, 37461, 14064, 36460, + 46434, 8387, 42174, 36763, + 49205, 48759, 10526, 3446, + 14035, 2195, 6798, 38782 +}; + +int table_cycle_size = 16; + +/* At each instruction, get a new X and Y from the tables to later + verify that we have not reused a value incorrectly. */ +void simulate_thread_other_threads () +{ + static int current = 0; + + if (++current >= table_cycle_size) + current = 0; + x = table1[current]; + y = table2[current]; +} + +/* Return true if error, otherwise 0. */ +int verify_result () +{ + /* [i] should not equal [j], because that would mean that we hoisted + [x] and [y] instead of loading them again. */ + int fail = i == j; + if (fail) + printf("FAIL: i (%u) should not equal j (%u)\n", i, j); + return fail; +} + +int simulate_thread_step_verify () +{ + return verify_result (); +} + +int simulate_thread_final_verify () +{ + return verify_result (); +} + +__attribute__((noinline)) +void simulate_thread_main() +{ + /* The values of x or y should not be hoisted across reads of + flag[12]. + + For example, when the second load below synchronizes with another + thread, the synchronization is with a release, and that release + may cause a stored value of x/y to be flushed and become visible. + So, for this case, it is incorrect for CSE/CSA/and-others to + hoist x or y above the load of flag2. */ + + /* Execute loads with value changing at various cyclic values. */ + if (__atomic_load_n (&flag1, __ATOMIC_ACQUIRE)) + i = x + y; + + if (__atomic_load_n (&flag2, __ATOMIC_ACQUIRE)) + a = 10; + j = x + y; + + /* Since x and y have been changing at each instruction above, i and j + should be different. If they are the same, we have hoisted + something incorrectly. */ +} + +main() +{ + simulate_thread_main (); + simulate_thread_done (); + return 0; +}