[PATCH] D50754: Implementation of a vtable interleaving algorithm

[Vlad Tsyrklevich via Phabricator via llvm-commits]

vlad.tsyrklevich added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO.h:238
 
+/// This pass interleaves vtables and lowers llvm.typ.test intrinsic.
+ModulePass *createInterleaveVTablesPass();
----------------
typ -> type


================
Comment at: llvm/include/llvm/Transforms/IPO/InterleaveVTables.h:2
+//===- InterleaveVTables.h - VTables interleaving pass -----------*- C++
+//-*-===//
+//
----------------
nit: Bad wrapping


================
Comment at: llvm/include/llvm/Transforms/IPO/InterleaveVTables.h:24
+#include <limits>
+#include <set>
+#include <vector>
----------------
These STL includes don't seem like they're required here.


================
Comment at: llvm/lib/Transforms/IPO/InterleaveVTables.cpp:2
+//===- InterleaveVTables.cpp - VTables interleaving pass
+//-------------------===//
+//
----------------
nit: more bad line wrapping


================
Comment at: llvm/lib/Transforms/IPO/InterleaveVTables.cpp:194
+
+  void getOffsetFromGlobalVaribleName(StringRef GlobalVaribleName,
+                                      int64_t &Offset);
----------------
nit: Variable*


================
Comment at: llvm/lib/Transforms/IPO/InterleaveVTables.cpp:345
+    GlobalVariable *InterleavedVTable) {
+  for (auto &TypdIdAndUserInfo : TypeIdUsers) {
+    Metadata *TypeId = TypdIdAndUserInfo.first;
----------------
nit: Typd -> Type


================
Comment at: llvm/lib/Transforms/IPO/InterleaveVTables.cpp:423
+
+  // Initialized UpperEntires and LowerEntries.
+  for (auto GV : OrderedGlobals) {
----------------
nit: -d


================
Comment at: llvm/lib/Transforms/IPO/InterleaveVTables.cpp:488
+                                                        : &LowerEntries;
+    while (UpperEntries.size() != LowerEntries.size()) {
+      CurList->push_back(ConstantPointerNull::get(Int8PtrTy));
----------------
nit: no brackets here


================
Comment at: llvm/lib/Transforms/IPO/InterleaveVTables.cpp:493
+
+  // Interleave the two vector
+  for (size_t I = 0; I < UpperEntries.size(); I++) {
----------------
nit: vectors


================
Comment at: llvm/lib/Transforms/IPO/InterleaveVTables.cpp:600
+//
+// OrderedGTMs: the result of all the vtables ordered
+// by a pre-order traversal of the disjoint hierarchy.
----------------
Should this be OrderedGlobals?


================
Comment at: llvm/test/Transforms/InterleaveVTables/imcomplete.ll:1
+; RUN: opt -S -interleavevtables < %s | FileCheck %s
+
----------------
File name typo: incomplete


================
Comment at: llvm/test/Transforms/InterleaveVTables/imcomplete.ll:9
+
+; The first list initilized with offset-to-top entries: [0, 4, 8]
+; The second list initilized with RTTI entries: [1, 5, 9]
----------------
initialized (here and the line below)


================
Comment at: llvm/test/Transforms/InterleaveVTables/vbase.ll:9
+
+; The first list initilized with offset-to-top entries: [0, 5, 11]
+; The second list initilized with RTTI entries: [1, 6, 12]
----------------
initialized (here and the line below)


https://reviews.llvm.org/D50754