[PATCH] D87879: [LoopInterchange] Add dominance check to guarantee output dependency order

[Florian Hahn via Phabricator via llvm-commits]

fhahn requested changes to this revision.
fhahn added a comment.
This revision now requires changes to proceed.

Thanks for the patch! Could you also add a test case where `src` dominates `dest`?



================
Comment at: llvm/test/Transforms/LoopInterchange/pr47523-implicit-out-dep-order.ll:3
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
----------------
does the test require any AArch64 specific cost modeling? Otherwise best to remove the triple, otherwise the test may fail when the AArch64 backend is not built.


================
Comment at: llvm/test/Transforms/LoopInterchange/pr47523-implicit-out-dep-order.ll:52
+for.cond2.preheader.preheader.i:
+  %i = sext i32 undef to i64
+  br label %for.cond2.preheader.i
----------------
could we just use `0` instead of `%I` as incoming values in the phi?


================
Comment at: llvm/test/Transforms/LoopInterchange/pr47523-implicit-out-dep-order.ll:71
+land.end.i:                                       ; preds = %land.rhs.i, %for.body4.i
+  br i1 icmp eq (i64 urem (i64 zext (i16 ptrtoint ([1 x %struct.a]* @c to i16) to i64), i64 4073709551606), i64 0), label %if.end.i, label %if.then.i
+
----------------
Is the condition here important? Could this instead just be an `i1` that is a function argument?


================
Comment at: llvm/test/Transforms/LoopInterchange/pr47523-implicit-out-dep-order.ll:74
+if.then.i:                                        ; preds = %land.end.i
+  %i2 = load i16, i16* bitcast (i32* @g to i16*), align 4, !tbaa !4
+  %inc.i = add i16 %i2, 1
----------------
could `@g` just be defined as `i16` or the load/store be widened to `i32`, so we do not need the bit casts?


================
Comment at: llvm/test/Transforms/LoopInterchange/pr47523-implicit-out-dep-order.ll:93
+h.exit:                                           ; preds = %for.cond.for.end14_crit_edge.i
+  %i3 = load i32, i32* @g, align 4, !tbaa !0
+  %call4 = tail call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64 0), i32 %i3)
----------------
is `!tbaa` needed for any memory operations here?


================
Comment at: llvm/test/Transforms/LoopInterchange/pr47523-implicit-out-dep-order.ll:94
+  %i3 = load i32, i32* @g, align 4, !tbaa !0
+  %call4 = tail call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64 0), i32 %i3)
+  ret i32 0
----------------
This seems unneeded for the test. The test could just return `%i3`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87879