[PATCH] D20483: [esan] EfficiencySanitizer working set tool fastpath

[Derek Bruening via llvm-commits]

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 ||
----------------
aizatsky wrote:
> 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
----------------
aizatsky wrote:
> 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.


http://reviews.llvm.org/D20483