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

Whitney Tsang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 5 12:00:26 PST 2021


Whitney marked an inline comment as done.
Whitney added inline comments.


================
Comment at: llvm/lib/Analysis/LoopNestAnalysis.cpp:19
 #include "llvm/Analysis/ValueTracking.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
 
----------------
dblaikie wrote:
> 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? 
opps, this include is no longer needed, I just forgot to remove it.
Thanks for pointing it out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93665



More information about the llvm-commits mailing list