[PATCH] D19167: [esan] EfficiencySanitizer instrumentation pass

[Mike Aizatsky via llvm-commits]

aizatsky added inline comments.

================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:306
@@ +305,3 @@
+    I->eraseFromParent();
+    Res = true;
+  } else
----------------
filcab wrote:
> 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.
No I do not have any strong preference.


http://reviews.llvm.org/D19167