[LLVMdev] memory scopes in atomic instructions

[Sahasrabuddhe, Sameer]

On 11/21/2014 9:01 AM, Sahasrabuddhe, Sameer wrote:
> Meanwhile, I will start working on the required changes. The only 
> decision required right now is whether to use "1" for SingleThread, or 
> the largest integer value.

I am currently trying to understand how the bitcode reader and writer 
work, and it will certainly take some time. Besides, that the SDNode 
class needs to be updated to use a separate unsigned field for the 
synchronization scope.

Meanwhile, here's a patch that shows the immediate effect of allowing 
more than two scopes. There seem to be only two users for now --- the 
X86 target and tsan. The patch attempts to fix them in slightly 
different ways:

 1. The X86 target understands only two scopes --- SingleThread or
    CrossThread (to be renamed later). All other "not SingleThread"
    scopes will be treated as CrossThread. I am not sure if this needs
    an assertion instead ... other scopes should not occur in the
    program anyway!

    Interestingly, this sort of "scope promotion" is supposed to be okay
    for OpenCL and will not introduce any new data races. The frontend
    can do this explicitly when translating OpenCL programs to targets
    with just two scopes.

 2. tsan has a similar problem, but it should definitely assert for
    unknown scopes. If it attempted to promote them instead, then it
    will hide data races that it was supposed to find! The assertion can
    be replaced if/when tsan learns how to model other scopes for a
    given target.

Sameer.

-------------- next part --------------
commit 847121c1ba4b49f459a7f72c7527b82435b377f9
Author: Sameer Sahasrabuddhe <sameer.sahasrabuddhe at amd.com>
Date:   Mon Nov 24 12:03:21 2014 +0530

    replace check for "CrossThread" with "not SingleThread"

diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp
index 47c8ce0..022a6e2 100644
--- a/lib/Target/X86/X86ISelLowering.cpp
+++ b/lib/Target/X86/X86ISelLowering.cpp
@@ -18736,7 +18736,7 @@ static SDValue LowerATOMIC_FENCE(SDValue Op, const X86Subtarget *Subtarget,
 
   // The only fence that needs an instruction is a sequentially-consistent
   // cross-thread fence.
-  if (FenceOrdering == SequentiallyConsistent && FenceScope == CrossThread) {
+  if (FenceOrdering == SequentiallyConsistent && FenceScope != SingleThread) {
     if (hasMFENCE(*Subtarget))
       return DAG.getNode(X86ISD::MFENCE, dl, MVT::Other, Op.getOperand(0));
 
diff --git a/lib/Transforms/Instrumentation/ThreadSanitizer.cpp b/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
index 417f2a1..f4cd8bd 100644
--- a/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ b/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -298,9 +298,9 @@ void ThreadSanitizer::chooseInstructionsToInstrument(
 
 static bool isAtomic(Instruction *I) {
   if (LoadInst *LI = dyn_cast<LoadInst>(I))
-    return LI->isAtomic() && LI->getSynchScope() == CrossThread;
+    return LI->isAtomic() && LI->getSynchScope() != SingleThread;
   if (StoreInst *SI = dyn_cast<StoreInst>(I))
-    return SI->isAtomic() && SI->getSynchScope() == CrossThread;
+    return SI->isAtomic() && SI->getSynchScope() != SingleThread;
   if (isa<AtomicRMWInst>(I))
     return true;
   if (isa<AtomicCmpXchgInst>(I))
@@ -539,8 +539,14 @@ bool ThreadSanitizer::instrumentAtomic(Instruction *I) {
     I->eraseFromParent();
   } else if (FenceInst *FI = dyn_cast<FenceInst>(I)) {
     Value *Args[] = {createOrdering(&IRB, FI->getOrdering())};
-    Function *F = FI->getSynchScope() == SingleThread ?
-        TsanAtomicSignalFence : TsanAtomicThreadFence;
+    unsigned SynchScope = FI->getSynchScope();
+    Function *F = NULL;
+    switch (SynchScope) {
+      case SingleThread: F = TsanAtomicSignalFence; break;
+      case CrossThread: F = TsanAtomicThreadFence; break;
+      default:
+        llvm_unreachable("unsupported synchronization scope");
+    }
     CallInst *C = CallInst::Create(F, Args);
     ReplaceInstWithInst(I, C);
   }