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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
