r198640 - CodeGen: Initial instrumentation based PGO implementation

[Chris Lattner]

On Jan 13, 2014, at 2:52 PM, Eric Christopher <echristo at gmail.com> wrote:

>>> 
>>>    -  // 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;
>>> 
>>> Is it really worth that much for code gen/optimization time to not just always
>>> have it? Also, do we really want instrumentation to change the cfg we emit?
>>> Also, it doesn't change in debug mode, just without optimization so that part
>>> of the comment isn't quite accurate :)
>> 
>> Well, we can't elide the block if we're profiling, since it won't be
>> empty in that case (the instrumentation code has to go somewhere!). It
>> looks like the original change to skip the empty block went in to clean
>> up -O0 CodeGen (r129652) but that made things messier for debugging and
>> for gcov (PR9796) so conditionalizing on OptimizationLevel > 0 was added
>> in r154420.
>> 
>> Presumably at anything above -O0 the backend will remove the empty block
>> anyway, so it's probably safe to remove this logic completely. Thoughts?
>> 
> 
> I agree, but sometimes Chris gets a little funny about these extra
> blocks. Chris?

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?

-Chris