[PATCH] D49220: [x86] Teach the EFLAGS copy lowering to handle much more complex control flow patterns including forks, merges, and even cyles.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc marked 2 inline comments as done.
chandlerc added a comment.

Thanks all, fixed the issues as requested. One longer-term thing remains with a FIXME, but landing this for now.



================
Comment at: llvm/lib/Target/X86/X86FlagsCopyLowering.cpp:539
+      // handle copies inside of cycles.
+      for (auto MII = (&UseMBB == &MBB && !VisitedBlocks.count(&UseMBB))
+                          ? std::next(CopyI->getIterator())
----------------
echristo wrote:
> The logic of these fused loop feels a bit convoluted, but I can't come up with an easy way to split it - can you?
I totally agree. I tried a bunch of ways of writing this better and they were all terrible.

I think I know how to clean this up, but it is a really invasive change. This whole thing is just in desperate need of refactoring. If we pull all this apart into separate functions, I think the loop will become more clear and it might become easy to split them apart.

I'll queue that up for a follow-up patch as I don't really want to try and do it while making this functional change and I'd like to get the SLH patch this enables landed. But it definitely should be addressed. I've left a FIXME for now.


Repository:
  rL LLVM

https://reviews.llvm.org/D49220