<div class="gmail_quote">On 30 January 2012 09:45, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com">kcc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Any feedback? </blockquote><div><br></div><div><div>--- test/Instrumentation/ThreadSanitizer/tsan_basic.ll<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 0)</div><div>+++ test/Instrumentation/ThreadSanitizer/tsan_basic.ll<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 0)</div>
<div>@@ -0,0 +1,17 @@</div><div>+; RUN: opt < %s -tsan -S | FileCheck %s</div><div>+</div><div>+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"</div>
<div>+target triple = "x86_64-unknown-linux-gnu"</div><div>+</div><div>+define i32 @read_4_bytes(i32* %a) {</div><div>+entry:</div><div>+ %tmp1 = load i32* %a, align 4</div><div>+ ret i32 %tmp1</div><div>+}</div>
<div>+; CHECK: @read_4_bytes</div><div>+; CHECK-NOT: ret i32</div><div>+; CHECK: __tsan_func_entry</div><div>+; CHECK: __tsan_read4</div><div>+; CHECK: __tsan_func_exit</div><div>+; CHECK: ret i32</div><div>+; CHECK: __tsan_init</div>
</div><div><br></div><div>I'm not a fan of this pile of CHECK statements. You could actually write out the IR that you expect to get out and preserve the order using CHECK-NEXT. Even if you don't want the matches to be too specific, it would still be clearer to see "CHECK-NEXT: call {{.*}} @__tsan_func_entry".</div>
<div><br></div><div><div>Index: lib/Transforms/Instrumentation/ThreadSanitizer.cpp</div><div>===================================================================</div><div>--- lib/Transforms/Instrumentation/ThreadSanitizer.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 0)</div>
<div>+++ lib/Transforms/Instrumentation/ThreadSanitizer.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 0)</div><div>@@ -0,0 +1,149 @@</div><div>+//===-- ThreadSanitizer.cpp - race detector ---------------------*- C++ -*-===//</div>
</div><div><br></div><div>Hey hey, this ruler is 81 characters long. :)</div><div><br></div><div><div>+bool ThreadSanitizer::runOnModule(Module &M) {</div><div>+ bool Res = false;</div><div>+ CurrentModule = &M;</div>
<div>+ TD = getAnalysisIfAvailable<TargetData>();</div><div>+ if (!TD)</div><div>+ return false;</div><div>+</div><div>+ for (Module::iterator F = M.begin(), E = M.end(); F != E; ++F) {</div><div>+ if (F->isDeclaration()) continue;</div>
<div>+ Res |= handleFunction(M, *F);</div><div>+ }</div><div>+ // We instrumented at least one function. Insert a call to __tsan_init().</div><div>+ if (Res) {</div><div>+ IRBuilder<> IRB(M.getContext());</div>
<div>+ Value *TsanInit = M.getOrInsertFunction("__tsan_init",</div><div>+ IRB.getVoidTy(), NULL);</div><div>+ appendToGlobalCtors(M, cast<Function>(TsanInit), 0);</div>
<div>+ }</div><div>+ return Res;</div><div>+}</div></div><div><br></div><div>You could write this as a FunctionPass; create the __tsan_... methods in doInitialization() (and store the necessary pointers in your object so that you needn't look them up each time you instrument a function), then do the instrumentation per-function, then in doFinalization either add the call to __tsan_init or clean up declarations you created in doInitialization().</div>
<div><br></div><div>+ else if (isa<CallInst>(BI))</div><div><div><div>+ HasCalls = true;</div></div></div><div><br></div><div>What about invokes?</div><div><br></div><div><div><div>+ // Instrument memory accesses.</div>
<div>+ for (size_t i = 0, n = LoadsAndStores.size(); i < n; ++i) {</div><div>+ Res |= instrumentLoadOrStore(LoadsAndStores[i]);</div><div>+ }</div></div></div><div><br></div><div>Why not just call instrumentLoadOrStore as you encounter them? It seems that the vector is just overhead.</div>
<div><br></div><div>Nick</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br></div><div>--kcc<div><div class="h5"><br><div><br><br><div class="gmail_quote">
On Wed, Jan 18, 2012 at 11:38 AM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello, <div><br></div><div>The proposed patch is the first step towards race detection built into LLVM/Clang. </div><div>
The tool will be similar to AddressSanitizer in the way it works: </div><div> 1. A simple instrumentation module in lib/Transforms/Instrumentation/ThreadSanitizer.cpp (this patch)</div>
<div> 2. -fthread-sanitizer flag in clang (next patch)</div><div> 3. A run-time library in projects/compiler-rt/lib/tsan (patches will follow).</div><div><br></div><div>The patch: <a href="http://codereview.appspot.com/5545054/" target="_blank">http://codereview.appspot.com/5545054/</a> (also attached). </div>
<div><br></div><div>Here are some links about the previous versions of ThreadSanitizer</div><div>- <a href="http://code.google.com/p/data-race-test/" target="_blank">http://code.google.com/p/data-race-test/</a> -- main project page</div>
<div>- <a href="http://code.google.com/p/data-race-test/wiki/CompileTimeInstrumentation" target="_blank">http://code.google.com/p/data-race-test/wiki/CompileTimeInstrumentation</a> - description of the LLVM-based prototype (we are not going to reuse that code, but will reuse the ideas). </div>
<div>- <a href="http://code.google.com/p/data-race-test/wiki/GccInstrumentation" target="_blank">http://code.google.com/p/data-race-test/wiki/GccInstrumentation</a> - description of the GCC-based prototype. </div><div>- <a href="http://dev.chromium.org/developers/how-tos/using-valgrind/threadsanitizer" target="_blank">http://dev.chromium.org/developers/how-tos/using-valgrind/threadsanitizer</a> - ThreadSanitizer for Chromium </div>
<div><div>- <a href="http://data-race-test.googlecode.com/files/ThreadSanitizer.pdf" target="_blank">http://data-race-test.googlecode.com/files/ThreadSanitizer.pdf</a> -- paper about Valgrind-based tool published at WBIA'09 </div>
- <a href="http://data-race-test.googlecode.com/files/ThreadSanitizerLLVM.pdf" target="_blank">http://data-race-test.googlecode.com/files/ThreadSanitizerLLVM.pdf</a> -- paper about LLVM-based tool published at RV'2011<br>
<div> <br><div>Thanks, </div></div></div><div><br></div><div>--kcc </div>
</blockquote></div><br></div></div></div></div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br>