[PATCH] D20914: [esan|cfrag] Compute the struct field access variance

[Mike Aizatsky via llvm-commits]

aizatsky added inline comments.

================
Comment at: lib/esan/cache_frag.cpp:62
@@ +61,3 @@
+
+// We evaluate how much difference between the access counts of the two adjacent
+// struct fields.
----------------
Before we move to the weight, let's discuss logarithm base for a while.

Obviously:

log(T, V1/V2) = log2(v1/v2) / log2(T)

So changing T would just change a constant factor for all measurements. Why not statically fix it?

Maybe just use log2(v1/v2) without base? you could use __builtin_clzll for that.


================
Comment at: lib/esan/cache_frag.cpp:63
@@ +62,3 @@
+// We evaluate how much difference between the access counts of the two adjacent
+// struct fields.
+// The formula for calculating the difference score is (log(T, V1/V2) - 1) * V1
----------------
As for weight, can you please share your thinking why do you need it?

================
Comment at: lib/esan/cache_frag.cpp:66
@@ +65,3 @@
+// if V1 is greater than V2.
+// * "- 1":  making the difference score 0 if the two counts are close in value,
+// * "* V2": added weight for how imporant the difference is.
----------------
Is there a "-1" now?

================
Comment at: lib/esan/cache_frag.cpp:68
@@ +67,3 @@
+// * "* V2": added weight for how imporant the difference is.
+static u64 computeDifferenceScore(u64 Val1, u64 Val2, u32 Threshold) {
+  u64 Ratio, Weight;
----------------
Let's rename Threshold to Base.

================
Comment at: lib/esan/cache_frag.cpp:73
@@ +72,3 @@
+    Threshold = 2;
+  if (Val1 > Val2) {
+    Ratio = (Val2 == 0) ? Val1 : (Val1 / Val2);
----------------
if (Val2 > Val1) { swap(Val1, Val2); }

You won't need the Weight then.

================
Comment at: lib/esan/cache_frag.cpp:80
@@ +79,3 @@
+  }
+  while (Ratio > Threshold) {
+    ++Score;
----------------
In any case I suggest you extract a simple FastLog function. 


http://reviews.llvm.org/D20914