The specific number of records loaded depends on the number of kinds, but the difference between the lazy and not lazy cases does not.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
As a maintainer of a downstream fork of LLVM with its own metadata kinds, I'm a big fan of this change. Until this patch:
- Whenever anyone added a metadata kind, either to upstream LLVM or to my downstream fork, they'd need to remember to increment the number in the check for 74 bitcode-reader - Number of Metadata records loaded.
- Whenever someone added a metadata kind to upstream LLVM and update this test, the upstream change to this test file would conflict with my downstream changes to the test file. Or, if there was no conflict because the number of records was the same, my downstream test would start failing, and I would need to open this test file and increment the count to account for my downstream metadata kinds.
This change makes this test less brittle and has it focus only on testing lazy loading, which is I believe what the original test author, @mehdi_amini, intended. (I'll also suggest @ostannard as a reviewer since he last incremented the count in this file.)
Anyway this looks great to me but I'll wait for more reviews.
llvm/test/ThinLTO/X86/lazyload_metadata.ll | ||
---|---|---|
14 | Never saw this syntax with cat and the two redirections before. Is this equivalent to the following? ; RUN: (llvm-lto -thinlto-action=import %t2.bc -thinlto-index=%t3.bc -stats 2>&1 | awk '{print "LAZY: " $0}' \ && llvm-lto -thinlto-action=import %t2.bc -thinlto-index=%t3.bc -stats -disable-ondemand-mds-loading 2>&1 awk '{print "NOTLAZY: " $0}') ; RUN: | FileCheck %s |
llvm/test/ThinLTO/X86/lazyload_metadata.ll | ||
---|---|---|
14 | Oops, yep. That's a lot nicer, thanks! |
Reverted because this broke the test on Windows (http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/21273/steps/test-check-all/logs/stdio). Is there any way I can run buildbot on a diff that's still in review? I'd like to debug this, but I don't have a Windows setup.
Never saw this syntax with cat and the two redirections before. Is this equivalent to the following?