[PATCH] D20892: [esan|cfrag] Instrument GEP instr for struct field access.

[Mike Aizatsky via llvm-commits]

aizatsky added inline comments.

================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:329
@@ +328,3 @@
+    // Remember the counter variable for each struct type.
+    StructTyMap.insert(std::pair<Type *, GlobalVariable *>(StructTy, Counters));
+
----------------
Will this grow and grow in huge lto programs? Is this a concern?

================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:613
@@ +612,3 @@
+  if (GepInst == nullptr || !isa<StructType>(GepInst->getSourceElementType()) ||
+      StructTyMap.count(GepInst->getSourceElementType()) == 0 ||
+      !GepInst->hasAllConstantIndices() ||
----------------
It is not clear to me why this map would contain this source element type at all. Wouldn't this work only for structs defined in the same module?

================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:616
@@ +615,3 @@
+      // Only handle simple struct field GEP.
+      GepInst->getNumIndices() != 2)
+    return false;
----------------
Do you need counters for how many accesses were skipped?


http://reviews.llvm.org/D20892