[PATCH] D93665: [LoopNest] Allow empty basic blocks without loops

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/Analysis/LoopNestAnalysis.cpp:19
 #include "llvm/Analysis/ValueTracking.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
 
----------------
While not, I think, technically impossible - this is a rather "interesting" dependency to add. (generally Analysis can't depend on Transforms, because Transforms depend on Analysis - though Transforms/Utils doesn't depend on Transforms, so we can have the diamond of Analysis depends on Transforms and Transforms and Analysis depend on Transforms/Utils)

But this is missing the CMakeLists.txt update to indicate that Analysis depends on TransformUtils.

While the dependency is technically possible, I think it might be better to revert this patch (& later ones in the series) before having a broader discussion (possibly on llvm-dev) about this new dependency due to its subtlety. Perhaps some utilities should move to Analysis or an Analysis/Utils, or an IR/Utils instead of living in Transforms/Utils, if they aren't actually doing transformations? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93665