[PATCH] D43423: [SimplifyCFG] Create flag to disable simplifyCFG.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D43423#1011170, @davide wrote:

> Some high level comments:
>
> 1. This is something that GCC does relatively frequently (adding frontend options to control optimization passes), but LLVM tends to not expose these details. FWIW, I'd very much prefer the details of the optimizer wouldn't be exposed as frontend flags.


We might not need the flag if we decide to always set `no_simplify_cfg` during codegen with coverage instrumentation.  But we're not sure if we want to do that yet.

> 2. I really don't think metadata is the correct way of conveying this information to the optimizer. Among all the other problems, metadata can be stripped willy-nilly. Maybe a function attribute will work better?

Should be simple enough to change this to a function attribute if you think that is more appropriate.  Wasn't sure whether metadata or an attribute made more sense.

> Some alternatives which I don't like, but I can't currently propose anything better are:
> 
> 1. Having a separate optimization pipeline that you use in these cases

This seems like a lot more work.

> 2. Having an instrumentation pass that annotates your CFG to prevent passes from destroying coverage signal. That said, I'm not really familiar to comment on whether this is feasible or not.

This patch allows us to annotate our functions with `no_simplify_cfg` to do what you're suggesting.  Writing another instrumentation pass seems like overkill.

> Please note that completely disabling SimplifyCFG will result in non-trivial performance hits (at least, in my experience), so you might want to evaluate this carefully.
>  @chandlerc might have thoughts on how to address this.

We're still evaluating this with mixed results.  On one hand, disabling `simplifyCFG` makes most of our libFuzzer tests pass with `-O2`, and libFuzzer seems to perform at the same executions/sec rate.  On the other hand, executions/sec doesn't necessarily translate to more coverage/sec or bugs found.



================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:109
 
 static cl::opt<bool> MergeCondStores(
     "simplifycfg-merge-cond-stores", cl::Hidden, cl::init(true),
----------------
vitalybuka wrote:
> why metadata instead of cl:opt like these?
We want to be able to avoid `simplifyCFG` when we pass `-fsanitize=fuzzer` to the Clang driver.  AFAICT, the frontend does not explicitly invoke `simplifyCFG`, so it can't control the options here.


https://reviews.llvm.org/D43423