[PATCH] asan: optimization experiments

Kostya Serebryany kcc at google.com
Thu Mar 12 13:26:00 PDT 2015


================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:926
@@ +925,3 @@
+  // negatives) and make the decision on the optimization.
+  uint32_t Exp = ClForceExperiment;
+
----------------
dvyukov wrote:
> kcc wrote:
> > Do I understand correctly that this patch does not set Exp to anything interesting by default,
> > and that will be added in the following patches? 
> Yes, this patch does not add any active experiments.
> No, there won't be following patches with experiments. There is no reason to commit any experiments. You implement an experiment locally, evaluate it and then either commit the optimization or commit nothing.
Then we misunderstood each other from the beginning. 
My idea was to actually commit everything to trunk and let *all* users use it for many months, 
and make it clear that if any experimental report fires they should let *us* know. 
I doubt you can get good enough coverage with a one-off experiment -- there are not too many bugs lying around in any single code base to have assuring statistics. 
This is also why I thought we need to have just one experiment (no experiment IDs and such) so that the code is simpler and there is no additional performance / code size penalty for passing an extra param

================
Comment at: lib/asan/asan_thread.h:121
@@ +120,3 @@
+  // Rathole to pass experiments mask from __asan_exp_loadN to __asan_loadN.
+  u32 report_experiment_;
+
----------------
dvyukov wrote:
> kcc wrote:
> > That's gross. Why not just pass an extra parameter?? 
> Why is __asan_load/store such a huge macro today and not a normal function?
> The only reason I see is that the function is performance critical and we don't want to obtain pc/bp/sp and call another function (__asan_exp_load/store). I've made the current implementation based on this.
> If the function is not performance critical, then we need wipe the entire macro and replace it with a function.
> 
> 
Yes, we certainly do not want to obtain pc/bp on the main path.
We probably can replace this macro with a force-inline function though. (A bit fragile, but I think we already rely on force inline elsewhere)
Or to be 100% safe put the function body into one macro and define two variants of functions using that body macro.

http://reviews.llvm.org/D8198

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list