[PATCH] asan: optimization experiments

[Dmitry Vyukov]

PTAL


================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:926
@@ +925,3 @@
+  // negatives) and make the decision on the optimization.
+  uint32_t Exp = ClForceExperiment;
+
----------------
kcc wrote:
> 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
As you wish. The actual experiment will be in a separate change then.

But I doubt there are many users who use tip clang _and_ run it on a huge codebase with lots of bugs _and_ who know about experiments _and_ will be willing to report back _and_ will alter their infrastructure to detect experimental crashes. And in the end an experiment will somehow end up in a clang release as well.

Experiment ID is a must, otherwise you will need to ask users to send you a full reproducer (something they won't be able to do), and then you will have to stare at the code trying to figure out what llvm byte code it produced and how asan pass handled it. You won't be able to just compile it, because it uses a custom build system and the bug is not reproducible without a non-trivial input.


================
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_;
+
----------------
kcc wrote:
> 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. 
> 
done

http://reviews.llvm.org/D8198

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