[PATCH] D20485: [esan] Add working set base runtime library

Filipe Cabecinhas via llvm-commits llvm-commits at lists.llvm.org
Wed May 25 06:41:30 PDT 2016


filcab added inline comments.

================
Comment at: lib/esan/working_set.cpp:53
@@ +52,3 @@
+  while (I < NumLines && (uptr)Shadow % 4 != 0) {
+    if ((*Shadow & ShadowAccessedVal) != ShadowAccessedVal)
+      *Shadow |= ShadowAccessedVal;
----------------
bruening wrote:
> aizatsky wrote:
> > I don't really see a reason to do an if() check here. Why not blindly "*Shadow |= ShadowAccessedVal"? Is it really faster? I would guess that having a branch is not very good for performance?
> The check is faster.  In every shadow tool we have ever made it is faster to load, compare, and only store if the value is not already there, than to perform a blind store.
> 
> 
I'd guess we end up having lots of shared cache locations. If we were writing all the time, then we'd end up with lots of cache/memory traffic for no reason, and the lines would be marked as exclusive all the time.
(Unless there are some cache optimizations where it doesn't change the state (any more than a read would) if the bit pattern written to it is the same. I'm not sure these exist)


Repository:
  rL LLVM

http://reviews.llvm.org/D20485





More information about the llvm-commits mailing list