[llvm-commits] [llvm] r162868 - /llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp

[Andrew Trick]

On Sep 7, 2012, at 4:57 PM, Alastair Murray <alastairmurray42 at gmail.com> wrote:

> Hi Andrew, Michael,
> 
> I believe this change (r162868) introduced an incorrect assert.
> 
> Please see the attached one line patch to remove it and associated test case (fails on svn trunk, passes with the assert removed).  I do not have commit access, so if the patch is okay then please commit it.
> 
> This assert trigger on 13 benchmarks in projects/test-suite when compiling with profiling data.
> 
> The test case was created by first letting bugpoint reduce a version of MultiSource/Applications/SPASS/SPASS which had branch weight metadata set by -profile-metadata-loader.  Then I further reduced it by hand (without a working simplifycfg bugpoint was limited).
> 
> I think the key to triggering the assert is to have partial weight metadata.  If the second branch in the test case is given metadata (even all 0 weights) then the assert does not trigger.  The metadata is missing due to an earlier pass not preserving it, but that is valid behaviour (though sub-optimal).

Committed r163437. Thanks for the test case!

Michael, since you've already created unit tests, it would be nice to have them checked into preserve-branchweights.ll too!

-Andy

> Regards,
> Alastair.
> 
> On 29/08/12 17:46, Andrew Trick wrote:
>> Author: atrick
>> Date: Wed Aug 29 16:46:38 2012
>> New Revision: 162868
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=162868&view=rev
>> Log:
>> Preserve branch profile metadata during switch formation.
>> 
>> Patch by Michael Ilseman!
>> This fixes SimplifyCFGOpt::FoldValueComparisonIntoPredecessors to preserve metata when folding conditional branches into switches.
> > *snip*
> 
> <simplify_cfg_weights_fix.patch>