[PATCH] D39267: [GISel]: Change Legalization from top down to bottom up + DCE

[Aditya Nandakumar via Phabricator via llvm-commits]

aditya_nandakumar marked 2 inline comments as done.
aditya_nandakumar added inline comments.


================
Comment at: include/llvm/CodeGen/GlobalISel/GISelWorkList.h:32
+  GISelWorkList(GISelWorkList &&) = default;
+  GISelWorkList &operator=(GISelWorkList &&) = default;
+
----------------
vsk wrote:
> Do you mean to delete the copy methods as well?
I don't really see the need to explicitly specify any special constructor here as I'm not doing anything special. I'm currently inclining towards only specifying the default constructor as that's the only use case for now.


================
Comment at: include/llvm/CodeGen/GlobalISel/GISelWorkList.h:41
+  void insert(MachineInstr *I) {
+    if (WorklistMap.insert(std::make_pair(I, Worklist.size())).second) {
+      Worklist.push_back(I);
----------------
vsk wrote:
> Use WorklistMap.try_emplace?
Updated.


================
Comment at: include/llvm/CodeGen/GlobalISel/GISelWorkList.h:46
+
+  // Remove - remove I from the worklist if it exists.
+  void remove(MachineInstr *I) {
----------------
vsk wrote:
> Use '///' for Doxygen support?
Updated.


https://reviews.llvm.org/D39267