[PATCH] D16404: [X86] Use hash table in LEA optimization pass

[Andrey Turetskiy via llvm-commits]

aturetsk added inline comments.

================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:42
@@ -41,3 +41,3 @@
 
 STATISTIC(NumSubstLEAs, "Number of LEA instruction substitutions");
 STATISTIC(NumRedundantLEAs, "Number of redundant LEA instructions removed");
----------------
Excluded from the patch.

================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:56
@@ +55,3 @@
+
+/// \brief Returns true if the instruction is LEA.
+static inline bool isLEA(const MachineInstr &MI);
----------------
Done.

================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:80
@@ +79,3 @@
+           (Other.Disp->isImm() || Other.Disp->isGlobal()) &&
+           "Address displacement operand is always an immediate or a global");
+
----------------
Done.

================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:88
@@ +87,3 @@
+         Disp->getGlobal() == Other.Disp->getGlobal()))
+      return true;
+
----------------
Done.

================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:97
@@ +96,3 @@
+  // Address' displacement operand.
+  const MachineOperand *Disp;
+};
----------------
I used hash_code inside the function and converted it into unsigned int since that's the return type expected from getHashValue.

================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:100
@@ +99,3 @@
+
+/// Provide DenseMapInfo for MemOpKey.
+namespace llvm {
----------------
Fixed.

================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:122
@@ +121,3 @@
+    assert(Val.Disp != PtrInfo::getTombstoneKey() &&
+           "Cannot hash the tombstone key");
+
----------------
```
assert((isLEA(MI) || MI.mayLoadOrStore()) &&
         "The instruction must be a LEA, a load or a store");
```
Is that OK?

================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:186
@@ -58,1 +185,3 @@
+  typedef DenseMap<MemOpKey, SmallVector<MachineInstr *, 16>> MemOpMap;
+
   /// \brief Returns a distance between two instructions inside one basic block.
----------------
Fixed.


http://reviews.llvm.org/D16404