r198640 - CodeGen: Initial instrumentation based PGO implementation

[Justin Bogner]

Chris Lattner <sabre at nondot.org> writes:
>>>>    -  // If the body of the case is just a 'break', and if there was no
>>>>    fallthrough,
>>>>    -  // try to not emit an empty block.
>>>>    -  if ((CGM.getCodeGenOpts().OptimizationLevel > 0) &&
>>>>    +  // If the body of the case is just a 'break', try to not emit an empty
>>>>    block.
>>>>    +  // If we're profiling or we're not optimizing, leave the block in for
>>>>    better
>>>>    +  // debug and coverage analysis.
>>>>    +  if (!CGM.getCodeGenOpts().ProfileInstrGenerate &&
>>>>    +      CGM.getCodeGenOpts().OptimizationLevel > 0 &&
>>>>           isa<BreakStmt>(S.getSubStmt())) {
>>>>         JumpDest Block = BreakContinueStack.back().BreakBlock;
>
> I don't like clang generating dead code.  This isn't good for -O0
> compile time because nothing removes them, and we get unnecessarily
> slow and bloated debug mode code.  However, if we need to drop a
> profiling counter in a block, that seems like a good reason to emit
> it.  Do we really need to do this though?  If the block would be
> elided by -O0 code, why do we care about its count?

In this particular case, we didn't elide the block at -O0 anyway. It's
only skipped if optimizations are on, which seems kind of silly since
the backend will remove it for us.

In general though, keeping the counter is useful for two reasons:

1. When we apply branch weights to the switch, we need a value for this
   case. Otherwise, the weights for all of the other cases will be
   skewed.
2. If we want to use this same instrumentation for coverage, we want to
   know how often this case triggers even though it doesn't do anything.