Patchwork [cxx-mem-model] disallow load data races (1 of some)

login
register
mail settings
Submitter Aldy Hernandez
Date March 29, 2011, 4:28 p.m.
Message ID <4D92089C.8080500@redhat.com>
Download mbox | patch
Permalink /patch/88808/
State New
Headers show

Comments

Aldy Hernandez - March 29, 2011, 4:28 p.m.
On 03/25/11 10:39, Richard Guenther wrote:
> On Fri, Mar 25, 2011 at 4:33 PM, Jakub Jelinek<jakub@redhat.com>  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.

Patch

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 <stdio.h>
-#include <stdlib.h>
-#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 <stdio.h>
-#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 <stdio.h>
-#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();
-}