Page MenuHomePhabricator

[Coroutine] Add statistics for the number of elided coroutine
ClosedPublic

Authored by ChuanqiXu on Jun 29 2021, 3:00 AM.

Details

Summary

Now we lack a benchmark to measure the performance change for each commit.
Since coro elide is the main optimization in coroutine module, I wonder it may be an estimation to count the number of elided coroutine in private code bases.
e.g., for a certain commit, if we found that the number of elided goes down, we could find it before the commit check-in.

Hopes this helps.

Diff Detail

Unit TestsFailed

TimeTest
110 msx64 debian > LLVM.Transforms/Coroutines::coro-elide.ll
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/Transforms/Coroutines/coro-elide.ll -S -passes='cgscc(repeat<2>(inline,function(coro-elide,dce)))' -stats | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/Transforms/Coroutines/coro-elide.ll
60 msx64 windows > LLVM.Transforms/Coroutines::coro-elide.ll
Script: -- : 'RUN: at line 3'; c:\ws\w3\llvm-project\premerge-checks\build\bin\opt.exe < C:\ws\w3\llvm-project\premerge-checks\llvm\test\Transforms\Coroutines\coro-elide.ll -S -passes='cgscc(repeat<2>(inline,function(coro-elide,dce)))' -stats | c:\ws\w3\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w3\llvm-project\premerge-checks\llvm\test\Transforms\Coroutines\coro-elide.ll

Event Timeline

ChuanqiXu created this revision.Jun 29 2021, 3:00 AM
ChuanqiXu requested review of this revision.Jun 29 2021, 3:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 29 2021, 3:00 AM

Cool! fix test failure?

ChuanqiXu updated this revision to Diff 355424.Jun 29 2021, 7:11 PM

Fix tests.

lxfind accepted this revision.Jun 29 2021, 8:13 PM
This revision is now accepted and ready to land.Jun 29 2021, 8:13 PM
thakis added a subscriber: thakis.Jun 30 2021, 7:08 AM

Hello :)

This makes Transforms/Coroutines/coro-elide.ll fail on mac/linux/windows in a PGO build. The following builds first build clang, then build clang with just-built clang with PGO instrumentation enabled, then run it to generate profiles, and then build things a third time with profiles (and LTO on linux and win). When running tests after the third build, this test fails:

https://ci.chromium.org/ui/p/chromium/builders/try/win_upload_clang/1646/overview => https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8843025821336802800/+/u/package_clang/stdout?format=raw

https://ci.chromium.org/ui/p/chromium/builders/try/mac_upload_clang/1786/overview => https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8843025821336802832/+/u/package_clang/stdout?format=raw

https://ci.chromium.org/ui/p/chromium/builders/try/linux_upload_clang/1634/overview => https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8843025821336802848/+/u/package_clang/stdout?format=raw

Pasting the failure output from the linux bot:

build/Release+Asserts/bin/clang
 llvm-lit: /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/utils/lit/lit/llvm/config.py:436: note: using clang: /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/clang
 llvm-lit: /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/utils/lit/lit/llvm/config.py:436: note: using ld.lld: /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/ld.lld
 llvm-lit: /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/utils/lit/lit/llvm/config.py:436: note: using lld-link: /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/lld-link
 llvm-lit: /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/utils/lit/lit/llvm/config.py:436: note: using ld64.lld: /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/ld64.lld
 llvm-lit: /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/utils/lit/lit/llvm/config.py:436: note: using wasm-ld: /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/wasm-ld
 llvm-lit: /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/utils/lit/tests/lit.cfg:89: warning: Setting a timeout per test not supported. Requires the Python psutil module but it could not be found. Try installing it via pip or via your operating system's package manager.
  Some tests will be skipped and the --timeout command line argument will not work.
 -- Testing: 84534 tests, 32 workers --
 Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70
 FAIL: LLVM :: Transforms/Coroutines/coro-elide.ll (61622 of 84534)
 ******************** TEST 'LLVM :: Transforms/Coroutines/coro-elide.ll' FAILED ********************
 Script:
 --
 : 'RUN: at line 3';   /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/opt < /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/test/Transforms/Coroutines/coro-elide.ll -S    -passes='cgscc(repeat<2>(inline,function(coro-elide,dce)))' -stats  2>&1 | /b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/FileCheck /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/test/Transforms/Coroutines/coro-elide.ll --check-prefixes=CHECK,STATS
 --
 Exit Code: 1
 
 Command Output (stderr):
 --
 /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/test/Transforms/Coroutines/coro-elide.ll:168:10: error: STATS: expected string not found in input
 ; STATS: 2 coro-elide - The # of coroutine get elided.
          ^
 <stdin>:105:10: note: scanning from here
  ret void
          ^
 <stdin>:125:1: note: possible intended match here
 attributes #2 = { nounwind readnone }
 ^
 
 Input file: <stdin>
 Check file: /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/test/Transforms/Coroutines/coro-elide.ll
 
 -dump-input=help explains the following input dump.
 
 Input was:
 <<<<<<
              .
              .
              .
            100:  %1 = bitcast i8* %0 to void (i8*)* 
            101:  call fastcc void %1(i8* %hdl) 
            102:  %2 = call i8* @llvm.coro.subfn.addr(i8* %hdl, i8 1) 
            103:  %3 = bitcast i8* %2 to void (i8*)* 
            104:  call fastcc void %3(i8* %hdl) 
            105:  ret void 
 check:168'0              X error: no match found
            106: } 
 check:168'0     ~~
            107:  
 check:168'0     ~
            108: ; Function Attrs: argmemonly nounwind readonly 
 check:168'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            109: declare token @llvm.coro.id(i32, i8* readnone, i8* nocapture readonly, i8*) #1 
 check:168'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            110:  
 check:168'0     ~
              .
              .
              .
            120: ; Function Attrs: nounwind 
 check:168'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~
            121: declare i1 @llvm.coro.alloc(token) #0 
 check:168'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            122:  
 check:168'0     ~
            123: attributes #0 = { nounwind } 
 check:168'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            124: attributes #1 = { argmemonly nounwind readonly } 
 check:168'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            125: attributes #2 = { nounwind readnone } 
 check:168'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 check:168'1     ?                                      possible intended match
 >>>>>>
 
 --
 
 ********************
 Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
 ********************
 Failed Tests (1):
   LLVM :: Transforms/Coroutines/coro-elide.ll

Please take a look, or revert for now if it takes a while to fix.

http://llvm-cs.pcc.me.uk/lib/Support/Statistic.cpp#43 says

static cl::opt<bool> Stats(
    "stats",
    cl::desc("Enable statistics output from program (available with Asserts)"),
    cl::Hidden);

so maybe this test fails in all builds that don't have assertions enabled?

Yes, repros easily locally if you build with assertions off (eg a normal release build). Reverted in db86e5c91477286b6432a75857edf012652c58d9 for now.

The -stats command line flag requires asserts, so either the test needs REQUIRES: asserts or the stats testing needs to be broken into a separate test.

Yes, repros easily locally if you build with assertions off (eg a normal release build). Reverted in db86e5c91477286b6432a75857edf012652c58d9 for now.

The -stats command line flag requires asserts, so either the test needs REQUIRES: asserts or the stats testing needs to be broken into a separate test.

Thanks for suggestion!