[PATCH] D71689: Improve LLVM existing loopinterchange

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a reviewer: fhahn.
fhahn added a comment.

Thanks for the patch! It would be great if you could update the title/description of the patch with details on *how* the patch changes the way loops are processed in loop interchange, rather than just stating the new behavior. That will be very helpful when reviewing the functional change.



================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:54
+enum LoopInterchangeResult {
+  NotLegal,      // Loops are not legal to interchange.
+  NotProfitable, // Loops are not profitable to interchange.
----------------
Could you submit this change as a separate one? It seems unrelated to changing the strategy for interchanging. Also, could you make LoopInterchangeResult an `enum class`?


================
Comment at: test/Transforms/LoopInterchange/interchangeable.ll:202
+
+define void @interchange_11() {
+; CHECK-LABEL: @interchange_11(
----------------
I would prefer to add those new test cases in a separate file, this one already tests too much IMO. Maybe something like three-level-nest-interchangeable.ll?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D71689