[llvm-commits] [PATCH] MemorySanitizer instrumentation pass.

[Evgeniy Stepanov]

One more look at the patch anyone please?
Sorry to bother, but it seems we are close to agreement, and I've got
a large backlog of changes on top of this patch which I did not merge
in during review to avoid confusion.


On Tue, Nov 20, 2012 at 6:59 PM, Evgeniy Stepanov <eugenis at google.com> wrote:
>
>
> ================
> Comment at: llvm/include/llvm/IRBuilder.h:270-273
> @@ -268,2 +269,6 @@
>
> +  IntegerType* getIntPtrTy(DataLayout *DL, unsigned AddrSpace = 0) {
> +    return Type::getIntNTy(Context, DL->getPointerSizeInBits(AddrSpace));
> +  }
> +
>    //===--------------------------------------------------------------------===//
> ----------------
> Chandler Carruth wrote:
>> Has this gone in as a separate change? If not, please just submit it, consider it reviewed. =]
> They are in, I just have not rebased the patch past that revision yet.
>
> ================
> Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:367-368
> @@ +366,4 @@
> +          getCleanShadow(ConvertedShadow), "_mscmp");
> +      Instruction *CheckTerm = SplitBlockAndInsertIfThen(cast<Instruction>(Cmp),
> +          /* Unreachable */ !ClKeepGoing, MS.ColdCallWeights);
> +
> ----------------
> Chandler Carruth wrote:
>> I think this indenting doesn't work too well. How about:
>>
>>         Instruction *CheckTerm
>>           = SplitBlockAndInsertIfThen(cast<Instruction>(Cmp),
>>                                       /* Unreachable */ !ClKeepGoing,
>>                                       MS.ColdCallWeights);
> Are you sure that "=" goes to the new line?
>
> ================
> Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:473-474
> @@ +472,4 @@
> +  ///
> +  /// OriginAddr = (ShadowAddr + OriginOffset) & ~3ULL
> +  ///            = Addr & (~ShadowAddr & ~3ULL) + OriginOffset
> +  Value *getOriginPtr(Value *Addr, IRBuilder<> &IRB) {
> ----------------
> Chandler Carruth wrote:
>> BTW, this is a great comment. =] It helps a lot for the reader.
> And it's also incorrect :)
> I'm not sure what else to say here. Do you mean it's too verbose and I should remove it?
>
>
> http://llvm-reviews.chandlerc.com/D56
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits