[PATCH] D75865: Introduce unify-loop-exits pass.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/UnifyLoopExits.cpp:32
+
+  void getAnalysisUsage(AnalysisUsage &AU) const {
+    AU.addRequired<LoopInfoWrapperPass>();
----------------
sameerds wrote:
> arsenm wrote:
> > This seem to ignore the existence of switches (and call_br, but we don't handle that at all). Should this add a dependency on LowerSwitch (and preserve it)? Also should add a switch test or two
> @nhaehnle had mentioned at some point that requiring LowerSwitch via AnalysisUsage is not reliable. The structurizer seems to be the only pass in LLVM that seems to do that, which does not help my confidence at all. 
Unreliable in what way? I know there was something like a  circular dependence issue in the context of one of the other passes, that might not exist here. I would still try to add it here, and this should probably error if it encounters a switch or something


================
Comment at: llvm/lib/Transforms/Utils/UnifyLoopExits.cpp:154
+      }
+      dbgs() << "\n";
+
----------------
sameerds wrote:
> arsenm wrote:
> > single quotes
> Does this really matter? Seems terribly pedantic to me, and would be surprised to see this in any coding standard. At least nothing specific showed up when I search in the LLVM, Linux and GNU coding standards.
It's not really important, but they're not identical. The char version does save a function call vs. the string 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75865