[PATCH] D20483: [esan] EfficiencySanitizer working set tool fastpath

Mike Aizatsky via llvm-commits llvm-commits at lists.llvm.org
Fri May 20 15:06:36 PDT 2016


aizatsky added inline comments.

================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:71
@@ +70,3 @@
+static const uint64_t ShadowOffs[4] = { // Indexed by scale
+  0x0000130000000000ull,
+  0x0000220000000000ull,
----------------
bruening wrote:
> aizatsky wrote:
> > uh-oh. These are really magical constants copied.
> > Unfortunately there's no proper way to share code between runtime / llvm. I talked with our cube and the idea is to introduce a separate header (e.g. esan_compile_interface.h) and put all shareable constants there. At least that would make it clear what you are sharing. At best - you can manually synchronize the file by copying. WDYT?
> Yes, this lack of ability to share is something I was surprised at in the other sanitizers.  I talked to kcc about it early on when starting this project and it sounded like copying a handful of constants was something everyone was willing to live with and that they did not plan to push for any better approach, so I followed suit here.
> 
> Sure, a separate header sounds good.  Is there a plan to do this for the other sanitizers?
> 
> I assume the header would go into include/llvm/Transforms/ and should probably follow the CamelCase naming.
> Sure, a separate header sounds good. Is there a plan to do this for the other sanitizers?

Everyone agrees that the way it is now is very painful and we need to try this. If a separate header works for you - we would definitely steer others in the same direction.

> I assume the header would go into include/llvm/Transforms/ and should probably follow the CamelCase naming.

Yes. Even if you don't decide to copy it to runtime, it would clearly document all inter-dependencies and will make updating easier.


http://reviews.llvm.org/D20483





More information about the llvm-commits mailing list