[PATCH] D19167: [esan] EfficiencySanitizer instrumentation pass
Filipe Cabecinhas via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 20 10:32:55 PDT 2016
filcab added inline comments.
================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:306
@@ +305,3 @@
+ I->eraseFromParent();
+ Res = true;
+ } else
----------------
bruening wrote:
> filcab wrote:
> > The three cases above can be written more succintly (and made easier to follow/spot (lack of) differences).
> > Check out `AddressSanitizer::instrumentMemIntrinsic`.
> >
> Reviewer ping-pong :) Please see the original version of this code in the first patchset, which matched what asan had, and the reviewer comment above asking to change it to the current form.
I have a very strong preference over the simpler (to read and (even more important: ) to spot differences) code.
Mike: Do you have a strong preference over this idiom?
`MemTransferIntrin` has been there, with only these two subclasses, since 2009.
I think it's better to have the simpler version, and eventually change it if we need.
btw, Derek: You can reduce the noise by using `isa<Mem...>`, and getting the arguments from a `MemIntrinsic`, like ASan does.
http://reviews.llvm.org/D19167
More information about the llvm-commits
mailing list