[PATCH] D20483: [esan] EfficiencySanitizer working set tool fastpath
Derek Bruening via llvm-commits
llvm-commits at lists.llvm.org
Tue May 24 15:28:03 PDT 2016
bruening added inline comments.
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:419
@@ +418,3 @@
+ const uint32_t TypeSize = DL.getTypeStoreSizeInBits(OrigTy);
+ // Bail to the slowpath if the access might touch multiple cache lines.
+ if (!(TypeSize == 8 ||
> This comment doesn't seem to correspond to the code.
> Isn't it "Bail to the slowpath for unaligned access"?
> For multiple cache lines, don't you want to compare TypeSize with cache line size?
A memory access that is aligned to its size is guaranteed to only touch one cache line. (So long as it's <= 64 bytes which seems a safe assumption as memcpy, etc. intrinsics are handled elsewhere: this is a single load or store.)
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:425
@@ +424,3 @@
+ // We inline instrumentation to set the corresponding shadow bits for
+ // each cache line touched by the application.
+ // We assume that we've already ruled out memory accesses that touch more
> This sentence is immediately contradicted by the next. "each cache line" vs "we've already ruled out".
The first sentence is talking about the tool's instrumentation in general. I will update it to remove any confusion.
More information about the llvm-commits