[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