[llvm-commits] [PATCH] [msan] Refactor default shadow propagation and origin tracking
Kostya Serebryany
kcc at google.com
Fri Dec 14 00:29:34 PST 2012
================
Comment at: lib/Transforms/Instrumentation/MemorySanitizer.cpp:942
@@ +941,3 @@
+ IRBuilder<> &IRB;
+ MemorySanitizerVisitor* MSV;
+ public:
----------------
"Type *Foo", not "Type* Foo"
================
Comment at: lib/Transforms/Instrumentation/MemorySanitizer.cpp:1012
@@ +1011,3 @@
+ return IRB.CreateIntCast(V, dstTy, false);
+ size_t srcSizeInBits = srcTy->isVectorTy() ?
+ srcTy->getVectorNumElements() * srcTy->getScalarSizeInBits() :
----------------
these 3 lines and the following 3 lines look like a candidate for a separate function
================
Comment at: lib/Transforms/Instrumentation/MemorySanitizer.cpp:937
@@ +936,3 @@
+ /// arguments are initialized.
+ template <bool CombineShadow, bool CombineOrigin>
+ class Combiner {
----------------
do you ever plan to have CombineOrigin==false?
================
Comment at: lib/Transforms/Instrumentation/MemorySanitizer.cpp:958
@@ +957,3 @@
+
+ if (CombineOrigin && ClTrackOrigins) {
+ assert(OpOrigin);
----------------
I wonder if we can/should move ClTrackOrigins outside of this class (e.g. make it a CTOR argument)
================
Comment at: lib/Transforms/Instrumentation/MemorySanitizer.cpp:978
@@ +977,3 @@
+
+ void Done(Instruction *I) {
+ if (CombineShadow) {
----------------
Do you need 'I' here?
Why IRB is not enough?
Or maybe pass I instead of IRB to the CTOR?
http://llvm-reviews.chandlerc.com/D215
More information about the llvm-commits
mailing list