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

[Derek Bruening via llvm-commits]

bruening added inline comments.

================
Comment at: lib/esan/working_set.cpp:34
@@ +33,3 @@
+// See the shadow byte layout description above.
+static const u32 TotalWorkingSetBitIdx = 7;
+static const u32 CurWorkingSetBitIdx = 0;
----------------
aizatsky wrote:
> Instead of bit fiddling, do you think a struct with bit fields would look better? 
Hmm, the issue is that all of the code (not just this CL, but code that uses the other 6 bits) operates on the bits in a way that is not very amenable to using separate named fields: samples at each bit level accumulate into the bit to the left, we would still be creating masks for word-level and other operations (rather than hoping the compiler turns several field references into a single mask), etc.  While a struct might help with this top-level documentation, I don't think it would easily help with the actual code, and the declared field names would not be referenced.

I know you haven't seen any other code operating on the shadow values yet, so how about this: let's hold that thought, and once you see some future code that operates on all the bits, if you have an idea for how to use named bitfields in the code we can revisit at that point?


http://reviews.llvm.org/D20485