From patchwork Tue Mar 29 16:28:12 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [cxx-mem-model] disallow load data races (1 of some) From: Aldy Hernandez X-Patchwork-Id: 88808 Message-Id: <4D92089C.8080500@redhat.com> To: Richard Guenther Cc: Jakub Jelinek , Jeff Law , Michael Matz , Andrew MacLeod , "Joseph S. Myers" , Richard Henderson , gcc-patches Date: Tue, 29 Mar 2011 11:28:12 -0500 On 03/25/11 10:39, Richard Guenther wrote: > On Fri, Mar 25, 2011 at 4:33 PM, Jakub Jelinek wrote: >> On Fri, Mar 25, 2011 at 09:30:22AM -0600, Jeff Law wrote: >>> I'm not going to chime in on this specific problem; however, it is worth >>> noting that many of the issues raised by the C++0x memory model also >>> affect the linux kernel. >> >> But what they are seeing are certainly store data races, not load races, >> because no hw they care about (or no hw at all?) detects the latter. >> Having options to avoid store data races is useful not just for C++0x memory >> model compliance and Linux kernel, but e.g. for OpenMP too. > > And we have in the past removed code that created store data races > anyway. There is nothing new here. > > As stated multiple times in the past things get "interesting" when you > look at non-x86 hardware. Agreed on all counts, with everyone :). I will desist from trying to disable speculative loads and load data races which affect no one. Below is a patch I am committing to remove all tests checking for load races. As Richi has mentioned, there are some interesting store data races on non-x86 hardware. I will be looking into those, particularly non contiguous bitfield issues on ppc, s390, and possibly alpha: stay tuned. Thanks for the comments. * gcc.dg/memmodel/d2.c: Remove. * gcc.dg/memmodel/d3.c: Remove. * gcc.dg/memmodel/global-hoist.c: Remove. Index: gcc.dg/memmodel/global-hoist.c =================================================================== --- gcc.dg/memmodel/global-hoist.c (revision 170745) +++ gcc.dg/memmodel/global-hoist.c (working copy) @@ -1,73 +0,0 @@ -/* { dg-do link } */ -/* { dg-options "-O2 --param allow-load-data-races=0" } */ -/* { dg-final { memmodel-gdb-test } } */ - -/* Verify that a load of a global is not hoisted out of a loop. This - can introduce a data race, and is dissallowed if - --param allow-load-data-races is 0. */ - -#include -#include -#include "memmodel.h" - -int global = 0; -int sum[5]; - - -/* The other thread in the system increments the value of global by - one each time it is executed. By accessing 'global' in a loop, we can - check if the load has been hoisted out of the loop by seeing if - iterations of the loop get the same or different values of - global. */ -void -memmodel_other_threads() -{ - global++; -} - -/* Nothing to verify at each step. */ -int -memmodel_step_verify() -{ - return 0; -} - -/* Every element of the array should have a different value of global if the - * load is left in the loop like it is supposed to. */ -int -memmodel_final_verify() -{ - int ret = 0; - int x, y; - - for (x = 0; x < 4; x++) - for (y = x + 1; y < 5; y++) - { - if (sum[x] == sum[y]) - { - printf("FAIL: sum[%d] and sum[%d] have the same value : %d\n", - x, y, sum[x]); - ret = 1; - } - } - return ret; -} - -/* 'global' is bumped by "the other thread" every insn. Test that all - elements of 'sum' are different, otherwise load of 'global' has - been hoisted. */ -void -test() -{ - int x; - - for (x=0; x< 5; x++) - sum[x] = global; -} - -int -main() -{ - test(); - memmodel_done(); -} Index: gcc.dg/memmodel/d2.c =================================================================== --- gcc.dg/memmodel/d2.c (revision 170745) +++ gcc.dg/memmodel/d2.c (working copy) @@ -1,60 +0,0 @@ -/* { dg-do link } */ -/* { dg-options "-O2 --param allow-load-data-races=0" } */ -/* { dg-final { memmodel-gdb-test } } */ - -#include -#include "memmodel.h" - -/* This is a variation on global-hoist.c where instead of a loop, we - store global into different elements of an array in straightline - code with an if condition. This will catch cases where commoning - should be disabled when --param allow-load-data-races is 0. */ - -/* Test the FALSE path in test. */ - -int global = 0; -int sum[4] = { 0, 0, 0, 0 }; - -void memmodel_other_threads() -{ - global++; -} - -int memmodel_step_verify() -{ - return 0; -} - -int memmodel_final_verify() -{ - int ret = 0; - int x, y; - for (x = 0; x < 3; x++) - for (y = x + 1; y < 4; y++) - { - if (sum[x] == sum[y]) - { - printf("FAIL: sum[%d] and sum[%d] have the same value : %d\n", - x, y, sum[x]); - ret = 1; - } - } -} - -/* Since global is always being increased by the 'other' thread, all - elements of sum should be different. (no cse) */ -int test (int y) -{ - sum[0] = global; - if (y) - sum[1] = global; - else - sum[2] = global; - sum[3] = global; -} - -int main() -{ - test(0); - memmodel_done(); -} Index: gcc.dg/memmodel/d3.c =================================================================== --- gcc.dg/memmodel/d3.c (revision 170745) +++ gcc.dg/memmodel/d3.c (working copy) @@ -1,61 +0,0 @@ -/* { dg-do link } */ -/* { dg-options "-O2 --param allow-load-data-races=0" } */ -/* { dg-final { memmodel-gdb-test } } */ - -#include -#include "memmodel.h" - -/* A variation on global-hoist.c where instead of a loop, we store - global into different elements of an array in straightline code - with an if condition. This will catch cases where commoning should - be disabled when --param allow-load-data-races is 0. */ - -/* Test the TRUE path in test. */ - -int global = 0; -int sum[4] = { 0, 0, 0, 0 }; - -void memmodel_other_threads() -{ - global++; -} - -int memmodel_step_verify() -{ - return 0; -} - -int memmodel_final_verify() -{ - int ret = 0; - int x, y; - for (x = 0; x < 3; x++) - for (y = x + 1; y < 4; y++) - { - if (sum[x] == sum[y]) - { - printf("FAIL: sum[%d] and sum[%d] have the same value : %d\n", - x, y, sum[x]); - ret = 1; - } - } -} - -/* Since global is always being increased by the 'other' thread, all - elements of sum should be different. (No CSE) */ -int test(int y) -{ - sum[0] = global; - if (y) - sum[1] = global; - else - sum[2] = global; - sum[3] = global; -} - - -int main() -{ - test(1); - memmodel_done(); -}