This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PR tree-optimization/52558]: RFC: questions on store data race


On 04/13/12 18:22, Boehm, Hans wrote:


-----Original Message-----
From: Aldy Hernandez [mailto:aldyh@redhat.com]
Sent: Thursday, April 12, 2012 3:12 PM
To: Richard Guenther
Cc: Andrew MacLeod; Boehm, Hans; gcc-patches; Torvald Riegel
Subject: [PR tree-optimization/52558]: RFC: questions on store data
race

Here we have a testcase that affects both the C++ memory model and
transactional memory.

[Hans, this is caused by the same problem that is causing the
speculative register promotion issue you and Torvald pointed me at].


In the following testcase (adapted from the PR), the loop invariant
motion pass caches a pre-existing value for g_2, and then performs a
store to g_2 on every path, causing a store data race:

int g_1 = 1;
int g_2 = 0;

int func_1(void)
{
    int l;
    for (l = 0; l<  1234; l++)
    {
      if (g_1)
        return l;
      else
        g_2 = 0;	<-- Store to g_2 should only happen if !g_1
    }
    return 999;
}

This gets transformed into something like:

	g_2_lsm = g_2;
	if (g_1) {
		g_2 = g_2_lsm;	// boo! hiss!
		return 0;
	} else {
		g_2_lsm = 0;
		g_2 = g_2_lsm;
	}

The spurious write to g_2 could cause a data race.
Presumably the g_2_lsm = g_2 is actually outside the loop?

Why does the second g_2 = g_2_lsm; get introduced? I would have expected it before the return. Was the example just over-abbreviated?

There is some abbreviation going on :). To be exact, this is what -O2 currently produces for the lim1 pass.


<bb 2>:
  pretmp.4_1 = g_1;
  g_2_lsm.6_12 = g_2;

<bb 3>:
  # l_13 = PHI <l_6(5), 0(2)>
  # g_2_lsm.6_10 = PHI <g_2_lsm.6_11(5), g_2_lsm.6_12(2)>
  g_1.0_4 = pretmp.4_1;
  if (g_1.0_4 != 0)
    goto <bb 7>;
  else
    goto <bb 4>;

<bb 4>:
  g_2_lsm.6_11 = 0;
  l_6 = l_13 + 1;
  if (l_6 != 1234)
    goto <bb 5>;
  else
    goto <bb 8>;

<bb 8>:
  # g_2_lsm.6_18 = PHI <g_2_lsm.6_11(4)>
  g_2 = g_2_lsm.6_18;
  goto <bb 6>;

<bb 5>:
  goto <bb 3>;

<bb 7>:
  # g_2_lsm.6_17 = PHI <g_2_lsm.6_10(3)>
  # l_19 = PHI <l_13(3)>
  g_2 = g_2_lsm.6_17;

<bb 6>:
  # l_2 = PHI <l_19(7), 999(8)>
  return l_2;

So yes, there seems to be another write to g_2 inside the else, but probably because we have merged some basic blocks along the way.


Other than that, this sounds right to me. So does Richard's flag-based version, which is the approach I would have originally expected. But that clearly costs you a register. It would be interesting to see how they compare.

I am working on the flag based approach.


Thanks to both of you.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]