[PATCH] D20541: [esan|cfrag] Create the cfrag variable for the runtime

[Derek Bruening via llvm-commits]

bruening added inline comments.

================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:234
@@ +233,3 @@
+  // };
+  // FIXME: expand CacheFragInfo.
+  auto *Int8PtrTy = Type::getInt8PtrTy(*Ctx);
----------------
Please elaborate in the comment: really it's not "expand" which implies we have something complete now: really, this unit name is not important and this is just a skeleton struct that is not finished.  What we really want is to pass pointers to counters this pass will create for each field of each class/struct in the program.  Maybe part of the confusion is that this CL is only passing something that seems general to every tool, which is why the other reviews all wanted to share fields and pass something for all tools.  In hindsight it may have been better to send a CL that has all of the cache frag data (the counters) that we want to pass, and nullptr for other tools.

================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:266
@@ +265,3 @@
+  // struct ToolInfo {
+  //   const char *UnitName;
+  // };
----------------
This seems to be a compromise that does not seem ideal: now we have two structs with the same field that is duplicated instead of shared.  I would say, either go back to the original CL and pass nullptr here, or share this field via a contained struct or an anonymous generated struct sharing that field code (that approach seems to have been tried and rejected though).


http://reviews.llvm.org/D20541