[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