-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
verifyFunction spends too much time validating !dbg metadata in +asserts builds #47056
Comments
I think Limiting assertions outside As for the issue at hand in |
A potentially related issue motivated another patch: https://reviews.llvm.org/D94576 Reid, do you happen to still have the file & flags to reproduce this? I tried with a recent Clang build & current main, but couldn't reproduce it by building PassBuilder.cpp |
Nevermind, this can be reproduced by building 8.91 s 12.7% 3.00 ms (anonymous namespace)::LoopVectorize::runOnFunction(llvm::Function&) I also get similar results with http://github.com/llvm/llvm-project/commit/8dfe819bcd23 reverted. |
mentioned in issue llvm/llvm-bugzilla-archive#48689 |
The Verifier is called anyway at the beginning and end of the Pass Pipeline, and some complex transformations like LoopVectorize call the Verifier anyway in order to report errors early. It can be argued that verifying metadata after vectorization is inessential, and not doing this can significantly improve compile-times on debug builds. With this patch, although the Verifier itself verifies metadata, change the calls to the Verifier in LoopVectorize to skip this check. Fixes llvm#47056.
Extended Description
I was profiling LLVM for other reasons and accidentally used a build with assertions enabled. What I discovered is that, if you compile PassBuilder.cpp with assertions, most of the time is spent in the verifier, mostly due to asserts in SLPVectorizer and LoopVectorizer. There are 187,486 samples overall, and 131,698 of them (70.2%) are inside calls to verifyFunction.
If you drill into what verifyFunction is doing, all the time is spent in recursive calls to visitMDNode. These calls are rooted in visitInstruction calls to visitMDNode:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/IR/Verifier.cpp#L4431
visitMDNode in turn validates each of its operands. This is undesirable when validating a function: location metadata points upwards, so validating one location will ultimately validate the entire metadata graph rooted in the compile unit. See the tree of calls in this screenshot:
https://reviews.llvm.org/file/data/u5ms43qqwl3aahxopjql/PHID-FILE-ysoomivlg3fixz52f4ny/visitmdnode-profile.png
The code I linked was added in http://github.com/llvm/llvm-project/commit/8dfe819bcd23, but I'm not sure when this regressed. We used to ship clang with assertions enabled, so we saw these non-linear assert compile time issues in the field as they arose. Now that we are no longer watching carefully, it seems like these issues can slip in.
Should we try to establish the invariant that llvm::verifyFunction runs in O(#instructions), or should we move all calls to verifyFunction to EXPENSIVE_CHECKS?
The text was updated successfully, but these errors were encountered: