[PATCH] D98263: [LoopInterchange] fix tightlyNested() in LoopInterchange legality

[Congzhe Cao via Phabricator via llvm-commits]

congzhe added a comment.

In D98263#2644895 <https://reviews.llvm.org/D98263#2644895>, @Whitney wrote:

> I am fine with this change now, but in the future we should try to
>
> 1. Use utilities in LoopNest as much as possible
> 2. Loosen the definition perfect loop nest in LoopNest
> 3. Make the transformation more generic to handle more cases, e.g. the newly added test `interchange_07`.

Thanks for the review! I've updated the patch accordingly.



================
Comment at: llvm/test/Transforms/LoopInterchange/not-interchanged-tightly-nested.ll:12
 @D = common global [100 x [100 x [100 x i32]]] zeroinitializer
+ at E = common global [100 x [100 x i64]] zeroinitializer
 
----------------
Whitney wrote:
> can we reuse the existing global arrays?
Thanks, now removed the newly added array `E` and reused the existing array `A`.


================
Comment at: llvm/test/Transforms/LoopInterchange/not-interchanged-tightly-nested.ll:108
+
+;; The following Loop is not considered tightly nested and is not interchanged.
+; CHECK: Not interchanging loops. Cannot prove legality.
----------------
Whitney wrote:
> Please add comment for why it is not considered tightly nested. 
> Is it because we expect the outer loop header to have a conditional branch?
Thanks, comments added accordingly.
It is because the outer loop header does not branch to the inner loop preheader, or the
inner loop header, or the outer loop latch. If we did loop interchange on `interchange_07`, it would crash since the transformation phase currently cannot properly handle such an IR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98263