ChuanqiXu added inline comments.
================
Comment at: clang/lib/Serialization/ASTReader.cpp:10176-10184
+ if (auto *TTP = dyn_cast<TemplateTypeParmDecl>(D))
+ return TTP->hasDefaultArgument() &&
+ !TTP->defaultArgumentWasInherited();
+ if (auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(D))
+ return NTTP->hasDefaultArgument() &&
+ !NTTP->defaultArgumentWasInherited();
+ auto *TTP = cast<TemplateTemplateParmDecl>(D);
----------------
urnathan wrote:
> Rather than use dyn_cast, can't you use the switch(getKind()) idiom of the
> original? That'll reduce the number of virtual calls won't it? Bonus points
> for passing getKind() into the lambda so the compiler has a chance of
> noticing a loop unswitching possibility?
`dyn_cast` is implemented as "isa<X>(Val) ? cast<X>(Val) : nullptr;". Maybe the
judgement here is what you says 'virtual call', isn't it? So it looks like the
switch version would be better. From the source code, the switch version
contains one call to getKind() and at most 3 comparison with int. But the
current version would contain 2 calls to `isa<>` and 3 calls to `cast<>` and 4
null check. So we might think the switch version is better.
But on the one hand, the compiler optimization should be able to eliminate the
redundant calls in current version. So the number of `getKind()` and comparison
should be able to be reduced. (I know there is specific optimization for
switch, but the cases is too small so that there is no big optimization space.)
On the other hand, the switch version would require much more lines of code, a
draft of it would be:
```
switch(D->getKind()) {
case TemplateTypeParmDecl:
auto *TTP = cast<TemplateTypeParmDecl>(D);
return TTP->hasDefaultArgument() &&
!TTP->defaultArgumentWasInherited();
case ....
}
```
I feel it is worse. And I can't believe the performance here would be
significant. Premature optimization is the root of all evil.
For the loop switching optimization, as I said, there number of calls to
`getKind()` wouldn't be larger. On the other hand, I don't think it is good to
do loop switching here for the judgement of getKind()... It is not worthy. The
loop is relatively big and the comparison is relatively cheap. It would pay
more to do loop unstitching for this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118437/new/
https://reviews.llvm.org/D118437
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
