[PATCH] CodeGen: Implement the hint value for dyn_cast as described in the Itanium C++ ABI.
Chandler Carruth
chandlerc at gmail.com
Sat Feb 2 14:04:34 PST 2013
Looks really good. Few nits here.
I would add a comment about memoization just so that if this shows up on a profile, we remember the discusion we had on IRC about it.
================
Comment at: lib/CodeGen/CGExprCXX.cpp:1730
@@ +1729,3 @@
+ I != E; ++I) {
+ if (I->Access == AS_public) { // Ignore non-public inheritance.
+ for (CXXBasePath::iterator J = I->begin(), JE = I->end(); J != JE; ++J) {
----------------
As Doug commented, a continue seems better here.
================
Comment at: lib/CodeGen/CGExprCXX.cpp:1717
@@ +1716,3 @@
+ CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,
+ /*DetectVirtual=*/false);
+
----------------
Why not detect virtual? Then the path building can short circuit, and you can just detect it and return the -1?
================
Comment at: lib/CodeGen/CGExprCXX.cpp:1737-1740
@@ +1736,6 @@
+
+ // Accumulate the base class offsets.
+ const ASTRecordLayout &Layout = Context.getASTRecordLayout(J->Class);
+ Offset +=
+ Layout.getBaseClassOffset(J->Base->getType()->getAsCXXRecordDecl());
+ }
----------------
If we're on our second path, you can skip this computation.
http://llvm-reviews.chandlerc.com/D364
More information about the cfe-commits
mailing list