[llvm-commits] [PATCH] [msan] Refactor default shadow propagation and origin tracking

[Evgeniy Stepanov]

================
Comment at: lib/Transforms/Instrumentation/MemorySanitizer.cpp:958
@@ +957,3 @@
+
+      if (CombineOrigin && ClTrackOrigins) {
+        assert(OpOrigin);
----------------
Kostya Serebryany wrote:
> I wonder if we can/should move ClTrackOrigins outside of this class (e.g. make it a CTOR argument)
I don't think we should. That would add an extra constructor argument (more code at every use location) without any visible benefit.

================
Comment at: lib/Transforms/Instrumentation/MemorySanitizer.cpp:978
@@ +977,3 @@
+
+    void Done(Instruction *I) {
+      if (CombineShadow) {
----------------
Kostya Serebryany wrote:
> Do you need 'I' here? 
> Why IRB is not enough?
> Or maybe pass I instead of IRB to the CTOR?
"I" is the instruction that combined shadow and origin values are associated with. In general, it can be different from IRB insert point, and there may be callers (not at the moment) that want to setup an IRB, maybe insert more instruction in it, before using the Combiner.


http://llvm-reviews.chandlerc.com/D215