[PATCH] [opt] Devirtualize the SymbolBody type hierarchy and start compacting its members into the base class.

[Rui Ueyama]

================
Comment at: COFF/Symbols.cpp:46-48
@@ +45,5 @@
+  if (LK != RK) {
+    // Normalize so that the smaller kind is on the left.
+    if (LK > RK)
+      return -Other->compare(this);
+
----------------
Move this before "if (LK != RK)" to reduce nesting level a bit.

================
Comment at: COFF/Symbols.cpp:145-146
@@ +144,4 @@
+  switch (kind()) {
+  case DefinedBitcodeKind:
+    return cast<DefinedBitcode>(this)->getRVA();
+  case DefinedAbsoluteKind:
----------------
getRVA should never be called for bitcode symbols. You can remove this "case" and DefinedBitcode::getRVA.

================
Comment at: COFF/Symbols.cpp:168-169
@@ +167,4 @@
+  switch (kind()) {
+  case DefinedBitcodeKind:
+    return cast<DefinedBitcode>(this)->getFileOff();
+  case DefinedAbsoluteKind:
----------------
Ditto

================
Comment at: COFF/Symbols.cpp:170-171
@@ +169,4 @@
+    return cast<DefinedBitcode>(this)->getFileOff();
+  case DefinedAbsoluteKind:
+    return cast<DefinedAbsolute>(this)->getFileOff();
+  case DefinedImportDataKind:
----------------
This can also be removed.

================
Comment at: COFF/Symbols.h:97-104
@@ -88,3 +96,10 @@
 
-private:
-  const Kind SymbolKind;
+  const unsigned SymbolKind : 8;
+  unsigned IsExternal : 1;
+
+  // This bit is used by the \c DefinedRegular subclass.
+  unsigned IsCOMDAT : 1;
+
+  // This bit is used by the \c DefinedBitcode subclass.
+  unsigned IsReplaceable : 1;
+
----------------
Does the use of bitfield here save space? I guess StringRef is 8 byte aligned on 64 bit platform, so we have 8 bytes for attributes. We have four fields, so we can use 1 byte for each.

================
Comment at: COFF/Symbols.h:98
@@ +97,3 @@
+  const unsigned SymbolKind : 8;
+  unsigned IsExternal : 1;
+
----------------
Can you use C++11 initializer?

  unsigned IsExternal : 1 = true;

================
Comment at: COFF/Symbols.h:114
@@ -97,3 +113,3 @@
 public:
-  Defined(Kind K) : SymbolBody(K) {}
+  Defined(Kind K, StringRef N = "") : SymbolBody(K, N) {}
 
----------------
Add explicit.

================
Comment at: COFF/Symbols.h:130
@@ +129,3 @@
+// Symbols defined via a COFF object file.
+class ObjectFileDefined : public Defined {
+  friend SymbolBody;
----------------
All other classes starts with Defined, so name this DefinedObject, DefinedFile or DefinedCOFF.

================
Comment at: COFF/Symbols.h:131
@@ +130,3 @@
+class ObjectFileDefined : public Defined {
+  friend SymbolBody;
+public:
----------------
Why do you need this?

================
Comment at: COFF/Symbols.h:163
@@ -136,1 +162,3 @@
+  uint64_t getRVA() { return (*Data)->getRVA() + Sym.getValue(); }
+
   bool isCOMDAT() { return IsCOMDAT; }
----------------
Remove blank line.

================
Comment at: COFF/Symbols.h:188
@@ -165,1 +187,3 @@
 private:
+  friend SymbolBody;
+
----------------
Do we need this?

http://reviews.llvm.org/D10792

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/