[PATCH] D79396: [BrachProbablityInfo] Set edge probabilities at once. NFC.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:995
+  assert(TotalNumerator <= BranchProbability::getDenominator() + Probs.size());
+  assert(TotalNumerator >= BranchProbability::getDenominator() - Probs.size());
+}
----------------
yrouban wrote:
> rnk wrote:
> > rnk wrote:
> > > This assert doesn't appear to hold:
> > > https://ci.chromium.org/p/chrome/builders/ci/ToTWin/6284
> > > Assertion failed: TotalNumerator >= BranchProbability::getDenominator() - Probs.size(), file C:\b\s\w\ir\cache\builder\src\third_party\llvm\llvm\lib\Analysis\BranchProbabilityInfo.cpp, line 995
> > > 
> > > I will gather more information.
> > These are the relevant values:
> > 
> > TotalNumerator 2147483624
> > BranchProbability::getDenominator() 2147483648
> > Probs.size() 21
> > 
> > This is the IR switch:
> > ```
> >   switch i32 %0, label %sw.epilog.i [
> >     i32 20, label %sw.bb20.i
> >     i32 1, label %sw.bb1.i
> >     i32 2, label %sw.bb2.i
> >     i32 3, label %sw.bb3.i
> >     i32 4, label %sw.bb4.i
> >     i32 5, label %sw.bb5.i
> >     i32 6, label %sw.bb6.i
> >     i32 11, label %sw.bb7.i
> >     i32 12, label %sw.bb8.i
> >     i32 13, label %sw.bb9.i
> >     i32 7, label %sw.bb10.i
> >     i32 9, label %"?SkColorTypeToGrColorType@@YA?AW4GrColorType@@W4SkColorType@@@Z.exit"
> >     i32 8, label %sw.bb12.i
> >     i32 10, label %"?SkColorTypeToGrColorType@@YA?AW4GrColorType@@W4SkColorType@@@Z.exit"
> >     i32 14, label %sw.bb14.i
> >     i32 15, label %sw.bb15.i
> >     i32 18, label %sw.bb16.i
> >     i32 19, label %sw.bb17.i
> >     i32 16, label %sw.bb18.i
> >     i32 17, label %sw.bb19.i
> >   ], !dbg !77, !prof !81
> > ```
> > I think it comes from this source construct:
> > https://source.chromium.org/chromium/chromium/src/+/master:third_party/skia/include/private/GrTypesPriv.h;drc=2f11470d7ad8963a9add116df64d2edd1b85d3a4;l=869?originalUrl=https:%2F%2Fcs.chromium.org%2F
> > Inlined into this function:
> > https://source.chromium.org/chromium/chromium/src/+/master:third_party/skia/src/image/SkSurface_Gpu.cpp;l=342?q=MakeRenderTarget&ss=chromium&originalUrl=https:%2F%2Fcs.chromium.org%2F
> > 
> > I will revert for now, since this prevents us from seeing other ToT issues on PGO builders, but feel free to reland with a fix.
> please give me the !prof branch_weights so I can reproduce.
<0xd3589a0> = !{!"branch_weights", i32 1, i32 1, i32 1, i32 1, i32 1, i32 451, i32 1, i32 12, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1, i32 1}

Dumping them produced this:
0x00000001 / 0x80000000 = 0.00%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x77c7e9e7 / 0x80000000 = 93.58%
0x004761ef / 0x80000000 = 0.22%
0x03333332 / 0x80000000 = 2.50%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%
0x004761ef / 0x80000000 = 0.22%


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79396/new/

https://reviews.llvm.org/D79396