[PATCH] D19821: [EarlyCSE] Port to use MemorySSA (disabled by default). NFC.

Chad Rosier via llvm-commits llvm-commits at lists.llvm.org
Mon May 2 14:43:04 PDT 2016


mcrosier added a comment.

A few nits in passing.


================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:312
@@ -303,2 +311,3 @@
   /// It uses the same generation count as loads.
-  typedef ScopedHashTable<CallValue, std::pair<Value *, unsigned>> CallHTType;
+  typedef ScopedHashTable<CallValue, std::pair<Instruction *, unsigned>>
+      CallHTType;
----------------
Can this be committed as a separate change?

================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:570
@@ +569,3 @@
+
+  // handle case of e.g. load atomic -> store unordered
+  if (LaterHeapGen == EarlierMA)
----------------
Please capitalize handle and add a period.

================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:581
@@ +580,3 @@
+  // method.
+  assert(EarlierGeneration != LaterGeneration);
+  return false;
----------------
Please add a descriptive && "Error message!".

================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:766
@@ -656,3 +765,3 @@
       // generation, replace this instruction.
-      std::pair<Value *, unsigned> InVal = AvailableCalls.lookup(Inst);
-      if (InVal.first != nullptr && InVal.second == CurrentGeneration) {
+      std::pair<Instruction *, unsigned> InVal = AvailableCalls.lookup(Inst);
+      if (InVal.first != nullptr &&
----------------
Separate commit.

================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:784
@@ -670,3 +783,3 @@
       AvailableCalls.insert(
-          Inst, std::pair<Value *, unsigned>(Inst, CurrentGeneration));
+          Inst, std::pair<Instruction *, unsigned>(Inst, CurrentGeneration));
       continue;
----------------
Separate commit.  Maybe typdef?


http://reviews.llvm.org/D19821





More information about the llvm-commits mailing list