[PATCH] D154205: [MachineLICM] Handle subloops

[JinGu Kang via Phabricator via llvm-commits]

jaykang10 added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineLICM.cpp:778
+          }
+        }
+
----------------
nikic wrote:
> Uff, this looks like a pretty big hack. It is viable to pass the loop as a parameter instead of temporarily changing "global" state?
um... if possible, I did not want to change a lot in this pass... but I agree it is big hack.
Let me try to pass the global ones as parameters.


================
Comment at: llvm/lib/CodeGen/MachineLICM.cpp:1320
+    if (!DT->dominates(PrevMI->getParent(), MI->getParent()))
+      continue;
+
----------------
nikic wrote:
> This seems to work around a larger issue. The problem is that CSEMap will get initialized for whichever preheader we happen to hoist into first. Thanks to this check, we will at least not make invalid replacements, but it means we will miss CSE opportunities if we are hoisting into any other preheader.
> 
> Probably the CSE map should be by preheader, instead of having only the one.
It is good point.
Let me try to keep multiple CSE maps for multiple preheaders.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154205/new/

https://reviews.llvm.org/D154205