From: Brian Norris <banorris@uci.edu>
Date: Fri, 6 Jul 2012 18:44:46 +0000 (-0700)
Subject: datarace: factor subtle clock-vector logic into a reusable function
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=1555554a743a69644432215413e0dad83a18a235;p=cdsspec-compiler.git

datarace: factor subtle clock-vector logic into a reusable function

These comparisons are performed many times throughout datarace.cc, featuring a
lot of code duplication without strong documentation. This factors the
duplicated code into a single function and documents that single function more
clearly.

Brian Demsky: please review this fix (it should make no actual logical change)
and take a look at the TODO I have inserted; I'm not sure if the TODO is
warranted.
---

diff --git a/datarace.cc b/datarace.cc
index da9a3fc..1040a3b 100644
--- a/datarace.cc
+++ b/datarace.cc
@@ -25,6 +25,21 @@ static uint64_t * lookupAddressEntry(void * address) {
 	return &basetable->array[((uintptr_t)address)&MASK16BIT];
 }
 
+/**
+ * Compares a current clock-vector/thread-ID pair with a clock/thread-ID pair
+ * to check the potential for a data race.
+ * @param clock1 The current clock vector
+ * @param tid1 The current thread; paired with clock1
+ * @param clock2 The clock value for the potentially-racing action
+ * @param tid2 The thread ID for the potentially-racing action
+ * @return true if the current clock allows a race with the event at clock2/tid2
+ */
+static bool clock_may_race(ClockVector *clock1, thread_id_t tid1,
+                           modelclock_t clock2, thread_id_t tid2)
+{
+	return tid1 != tid2 && clock2 != 0 && clock1->getClock(tid2) <= clock2;
+}
+
 static void expandRecord(uint64_t * shadow) {
 	uint64_t shadowval=*shadow;
 
@@ -61,7 +76,7 @@ void fullRaceCheckWrite(thread_id_t thread, uint64_t * shadow, ClockVector *curr
 		modelclock_t readClock = record->readClock[i];
 		thread_id_t readThread = record->thread[i];
 
-		if (readThread != thread && readClock != 0 && currClock->getClock(readThread) <= readClock) {
+		if (clock_may_race(currClock, thread, readClock, readThread)) {
 			/* We have a datarace */
 			reportDataRace();
 		}
@@ -72,7 +87,7 @@ void fullRaceCheckWrite(thread_id_t thread, uint64_t * shadow, ClockVector *curr
 	modelclock_t writeClock = record->writeClock;
 	thread_id_t writeThread = record->writeThread;
 
-	if (writeThread != thread && writeClock != 0 && currClock->getClock(writeThread) <= writeClock) {
+	if (clock_may_race(currClock, thread, writeClock, writeThread)) {
 		/* We have a datarace */
 		reportDataRace();
 	}
@@ -108,7 +123,7 @@ void raceCheckWrite(thread_id_t thread, void *location, ClockVector *currClock)
 	modelclock_t readClock = READVECTOR(shadowval);
 	thread_id_t readThread = int_to_id(RDTHREADID(shadowval));
 
-	if (readThread != thread && readClock != 0 && currClock->getClock(readThread) <= readClock) {
+	if (clock_may_race(currClock, thread, readClock, readThread)) {
 		/* We have a datarace */
 		reportDataRace();
 	}
@@ -118,7 +133,7 @@ void raceCheckWrite(thread_id_t thread, void *location, ClockVector *currClock)
 	modelclock_t writeClock = WRITEVECTOR(shadowval);
 	thread_id_t writeThread = int_to_id(WRTHREADID(shadowval));
 
-	if (writeThread != thread && writeClock != 0 && currClock->getClock(writeThread) <= writeClock) {
+	if (clock_may_race(currClock, thread, writeClock, writeThread)) {
 		/* We have a datarace */
 		reportDataRace();
 	}
@@ -133,7 +148,7 @@ void fullRaceCheckRead(thread_id_t thread, uint64_t * shadow, ClockVector *currC
 	modelclock_t writeClock = record->writeClock;
 	thread_id_t writeThread = record->writeThread;
 
-	if (writeThread != thread && writeClock != 0 && currClock->getClock(writeThread) <= writeClock) {
+	if (clock_may_race(currClock, thread, writeClock, writeThread)) {
 		/* We have a datarace */
 		reportDataRace();
 	}
@@ -146,6 +161,8 @@ void fullRaceCheckRead(thread_id_t thread, uint64_t * shadow, ClockVector *currC
 		modelclock_t readClock = record->readClock[i];
 		thread_id_t readThread = record->thread[i];
 
+		/* TODO: should this have different logic than all the other
+		 * 'clock_may_race' calls? */
 		if (readThread != thread && currClock->getClock(readThread) <= readClock) {
 			/* Still need this read in vector */
 			if (copytoindex!=i) {
@@ -201,7 +218,7 @@ void raceCheckRead(thread_id_t thread, void *location, ClockVector *currClock) {
 	modelclock_t writeClock = WRITEVECTOR(shadowval);
 	thread_id_t writeThread = int_to_id(WRTHREADID(shadowval));
 
-	if (writeThread != thread && writeClock != 0 && currClock->getClock(writeThread) <= writeClock) {
+	if (clock_may_race(currClock, thread, writeClock, writeThread)) {
 		/* We have a datarace */
 		reportDataRace();
 	}
@@ -209,7 +226,7 @@ void raceCheckRead(thread_id_t thread, void *location, ClockVector *currClock) {
 	modelclock_t readClock = READVECTOR(shadowval);
 	thread_id_t readThread = int_to_id(RDTHREADID(shadowval));
 
-	if (readThread != thread && readClock != 0 && currClock->getClock(readThread) <= readClock) {
+	if (clock_may_race(currClock, thread, readClock, readThread)) {
 		/* We don't subsume this read... Have to expand record. */
 		expandRecord(shadow);
 		fullRaceCheckRead(thread, shadow, currClock);