[PATCH] D29865: [PDSE] Add a no-op pass.

[Davide Italiano via Phabricator via llvm-commits]

davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.

Have you considered integrating this in the current DSE infrastructure instead of having a separate pass?



================
Comment at: include/llvm/Analysis/MemoryLocation.h:121-122
   }
+
+  operator bool() const { return Ptr; }
 };
----------------
This seems to be unrelated. Split?


================
Comment at: include/llvm/Transforms/Scalar/PDSE.h:1-8
+#ifndef LLVM_TRANSFORMS_SCALAR_PDSE_H
+#define LLVM_TRANSFORMS_SCALAR_PDSE_H
+
+#include "llvm/IR/Function.h"
+#include "llvm/IR/PassManager.h"
+
+namespace llvm {
----------------
This needs copyright + doxygen comment as every other pass in LLVM.


================
Comment at: lib/Transforms/Scalar/CMakeLists.txt:48-49
   NewGVN.cpp
   PartiallyInlineLibCalls.cpp
+  PDSE.cpp
   PlaceSafepoints.cpp
----------------
ASCII Unsorted, I think (here and everywhere else, D < a)


================
Comment at: lib/Transforms/Scalar/PDSE.cpp:1-29
+// The pass implemented in this file performs partial dead store elimination as
+// described in:
+//
+//   Register Promotion Sparse Partial Redundancy Elimination of Loads and Store
+//   https://doi.org/10.1145/277650.277659
+//
+// "Partial" refers to the ability to transform partial store redundancies into
----------------
Thanks for the paper references. This needs to be changed to be similar to every other cpp file in LLVM (copyright etc..)


================
Comment at: lib/Transforms/Scalar/PDSE.cpp:40-41
+
+// TODO: Require BreakCriticalEdges
+
+using namespace llvm;
----------------
This `TODO` really belongs here or you can throw it away?


Repository:
  rL LLVM

https://reviews.llvm.org/D29865