From cff60c1409e36079b4bc6ecbda84565143bf00af Mon Sep 17 00:00:00 2001 From: Kostya Serebryany Date: Tue, 10 Apr 2012 22:29:17 +0000 Subject: [PATCH] [tsan] two more compile-time optimizations: - don't isntrument reads from constant globals. Saves ~1.5% of instrumented instructions on CPU2006 (counting static instructions, not their execution). - don't insrument reads from vtable (which is a global constant too). Saves ~5%. I did not measure the run-time impact of this, but it is certainly non-negative. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@154444 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Instrumentation/ThreadSanitizer.cpp | 53 ++++++++++++---- .../ThreadSanitizer/read_from_global.ll | 61 +++++++++++++++++++ 2 files changed, 103 insertions(+), 11 deletions(-) create mode 100644 test/Instrumentation/ThreadSanitizer/read_from_global.ll diff --git a/lib/Transforms/Instrumentation/ThreadSanitizer.cpp b/lib/Transforms/Instrumentation/ThreadSanitizer.cpp index bbadf1a9c90..8bb337eb2b0 100644 --- a/lib/Transforms/Instrumentation/ThreadSanitizer.cpp +++ b/lib/Transforms/Instrumentation/ThreadSanitizer.cpp @@ -58,6 +58,8 @@ struct ThreadSanitizerStats { size_t NumOmittedReadsBeforeWrite; size_t NumAccessesWithBadSize; size_t NumInstrumentedVtableWrites; + size_t NumOmittedReadsFromConstantGlobals; + size_t NumOmittedReadsFromVtable; }; /// ThreadSanitizer: instrument the code in module to find races. @@ -72,6 +74,7 @@ struct ThreadSanitizer : public FunctionPass { private: void choseInstructionsToInstrument(SmallVectorImpl &Local, SmallVectorImpl &All); + bool addrPointsToConstantData(Value *Addr); TargetData *TD; OwningPtr BL; @@ -145,11 +148,44 @@ bool ThreadSanitizer::doFinalization(Module &M) { << "; vt " << stats.NumInstrumentedVtableWrites << "; bs " << stats.NumAccessesWithBadSize << "; rbw " << stats.NumOmittedReadsBeforeWrite + << "; rcg " << stats.NumOmittedReadsFromConstantGlobals + << "; rvt " << stats.NumOmittedReadsFromVtable << "\n"; } return true; } +static bool isVtableAccess(Instruction *I) { + if (MDNode *Tag = I->getMetadata(LLVMContext::MD_tbaa)) { + if (Tag->getNumOperands() < 1) return false; + if (MDString *Tag1 = dyn_cast(Tag->getOperand(0))) { + if (Tag1->getString() == "vtable pointer") return true; + } + } + return false; +} + +bool ThreadSanitizer::addrPointsToConstantData(Value *Addr) { + // If this is a GEP, just analyze its pointer operand. + if (GetElementPtrInst *GEP = dyn_cast(Addr)) + Addr = GEP->getPointerOperand(); + + if (GlobalVariable *GV = dyn_cast(Addr)) { + if (GV->isConstant()) { + // Reads from constant globals can not race with any writes. + stats.NumOmittedReadsFromConstantGlobals++; + return true; + } + } else if(LoadInst *L = dyn_cast(Addr)) { + if (isVtableAccess(L)) { + // Reads from a vtable pointer can not race with any writes. + stats.NumOmittedReadsFromVtable++; + return true; + } + } + return false; +} + // Instrumenting some of the accesses may be proven redundant. // Currently handled: // - read-before-write (within same BB, no calls between) @@ -173,11 +209,16 @@ void ThreadSanitizer::choseInstructionsToInstrument( WriteTargets.insert(Store->getPointerOperand()); } else { LoadInst *Load = cast(I); - if (WriteTargets.count(Load->getPointerOperand())) { + Value *Addr = Load->getPointerOperand(); + if (WriteTargets.count(Addr)) { // We will write to this temp, so no reason to analyze the read. stats.NumOmittedReadsBeforeWrite++; continue; } + if (addrPointsToConstantData(Addr)) { + // Addr points to some constant data -- it can not race with any writes. + continue; + } } All.push_back(I); } @@ -236,16 +277,6 @@ bool ThreadSanitizer::runOnFunction(Function &F) { return Res; } -static bool isVtableAccess(Instruction *I) { - if (MDNode *Tag = I->getMetadata(LLVMContext::MD_tbaa)) { - if (Tag->getNumOperands() < 1) return false; - if (MDString *Tag1 = dyn_cast(Tag->getOperand(0))) { - if (Tag1->getString() == "vtable pointer") return true; - } - } - return false; -} - bool ThreadSanitizer::instrumentLoadOrStore(Instruction *I) { IRBuilder<> IRB(I); bool IsWrite = isa(*I); diff --git a/test/Instrumentation/ThreadSanitizer/read_from_global.ll b/test/Instrumentation/ThreadSanitizer/read_from_global.ll new file mode 100644 index 00000000000..a08453ac4a9 --- /dev/null +++ b/test/Instrumentation/ThreadSanitizer/read_from_global.ll @@ -0,0 +1,61 @@ +; RUN: opt < %s -tsan -S | FileCheck %s +; Check that tsan does not instrument reads from constant globals. + +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-S128" + +@const_global = external constant i32 +define i32 @read_from_const_global() nounwind uwtable readnone { +entry: + %0 = load i32* @const_global, align 4 + ret i32 %0 +} +; CHECK: define i32 @read_from_const_global +; CHECK-NOT: __tsan +; CHECK: ret i32 + +@non_const_global = global i32 0, align 4 +define i32 @read_from_non_const_global() nounwind uwtable readonly { +entry: + %0 = load i32* @non_const_global, align 4 + ret i32 %0 +} + +; CHECK: define i32 @read_from_non_const_global +; CHECK: __tsan_read +; CHECK: ret i32 + +@const_global_array = external constant [10 x i32] +define i32 @read_from_const_global_array(i32 %idx) nounwind uwtable readnone { +entry: + %idxprom = sext i32 %idx to i64 + %arrayidx = getelementptr inbounds [10 x i32]* @const_global_array, i64 0, i64 %idxprom + %0 = load i32* %arrayidx, align 4 + ret i32 %0 +} + +; CHECK: define i32 @read_from_const_global_array +; CHECK-NOT: __tsan +; CHECK: ret i32 + +%struct.Foo = type { i32 (...)** } +define void @call_virtual_func(%struct.Foo* %f) uwtable { +entry: + %0 = bitcast %struct.Foo* %f to void (%struct.Foo*)*** + %vtable = load void (%struct.Foo*)*** %0, align 8, !tbaa !3 + %1 = load void (%struct.Foo*)** %vtable, align 8 + call void %1(%struct.Foo* %f) + ret void +} + +; CHECK: define void @call_virtual_func +; CHECK: __tsan_read +; CHECK: = load +; CHECK-NOT: __tsan_read +; CHECK: = load +; CHECK: ret void + +!0 = metadata !{metadata !"int", metadata !1} +!1 = metadata !{metadata !"omnipotent char", metadata !2} +!2 = metadata !{metadata !"Simple C/C++ TBAA", null} +!3 = metadata !{metadata !"vtable pointer", metadata !2} +