[PATCH] D104182: [clang][NFC] Add IsAnyDestructorNoReturn field to CXXRecord instead of calculating it on demand

Sat Jun 12 13:15:02 PDT 2021

zero9178 added a comment.

In D104182#2815397 <https://reviews.llvm.org/D104182#2815397>, @davrec wrote:

> Was this performance hit when using the static analyzer?  A quick search suggests `isAnyDestructorNoReturn()` is only called within the analyzer, whereas comparable CXXRecordDecl methods whose results are stored (`hasIrrelevantDestructor()` etc.) seem to be called somewhere by Sema.
>
> So non-users of the analyzer would not benefit from this change, and will incur a slight cost, IIUC.  Is that cost remotely noticeable?  Probably not, but a quick test along those lines would be helpful.
>
> All in all this is probably good and advisable.

The only place where it is called in the static analyzer is in ExprEngineCXX.cpp:650. I was doing a normal compilation of a C++ file of mine, and they were coming from Analysis/CFG.cpp:1871 in `addAutomaticObjDtors` as well as CFG.cpp:4836 in `VisitCXXBindTemporaryForDtors`. So depending on the contents of the users C++ file it could improve their compilation speed as well. I suspect that in my case it was producing such a deep stacktrace due to a template class that is using a lot of layers of inheritance to preserve triviallity of constructors and more, tho that is just a guess. Nevertheless I'd wager it will more than likely improve the compile time for some other people as well.

As another test I just compared the compilation of X86ISelLowering.cpp from LLVM using clang-cl trunk, with and without this patch and could not measure any difference but margin of error. So it's definitely not guaranteed to be an improvement, but least isn't any worse.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104182