[PATCH] D118102: [LoopInterchange] Detect output dependency of a store instruction with itself

[Congzhe Cao via Phabricator via llvm-commits]

congzhe added a comment.





================
Comment at: llvm/test/Transforms/LoopInterchange/lcssa-preheader.ll:5
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
-
-; void foo(int n, int m) {
----------------
bmahjour wrote:
> This is a valid test case to interchange, and looks like after this patch, it won't be interchanged anymore. I believe @congzhe is looking into seeing if we can treat scalar output dependencies as interchange-preventing so that we can still get scenarios like this one.
Thanks Bardia, likely I was not clear enough during our last discussion -- just to clarify the fact that this test case wont be interchanged after this patch is due to the insufficient capability of DependenceAnalysis/Delinearization which gives [* *] as the direction vector. Remember that we now detect output dependency and take the store instruction into consideration, which results in a direction vector of [* *]. IIUC this problem seems independent of the current patch -- this patch fixes the deficiency that output dependencies are not considered, if it exposes other problems in other aspects I'm thinking to fix them in other patches maybe?

Previously I thought to remove this test since it seems not very meaningful. But actually with a trivial change in the test (similar to the trival changes in the two other tests in this patch), DependenceAnalysis could work properly and this test can be interchanged. I'll update the patch accordingly. 

-----

Regarding your comment that `if we can treat scalar output dependencies as interchange-preventing so that we can still get scenarios like this one`: 

I would like to first clarify again that scalar dependency is not related to this test/scenario, what fails this test is that dependence analysis gives [* *] as the direction vector.

I've also followed your suggestion and checked that if we treat scalar output dependencies as interchange-preventing, we would fail the following 10 tests. I looked into a few of them and IMHO it seems that scalar output dependencies seem to be legal in terms of interchange. Please correct me if I'm wrong.

Failed Tests (10):
  LLVM :: Transforms/LoopInterchange/inner-only-reductions.ll
  LLVM :: Transforms/LoopInterchange/innermost-latch-uses-values-in-middle-header.ll
  LLVM :: Transforms/LoopInterchange/interchange-no-deps.ll
  LLVM :: Transforms/LoopInterchange/lcssa.ll
  LLVM :: Transforms/LoopInterchange/outer-header-jump-to-inner-latch.ll
  LLVM :: Transforms/LoopInterchange/phi-ordering.ll
  LLVM :: Transforms/LoopInterchange/pr43176-move-to-new-latch.ll
  LLVM :: Transforms/LoopInterchange/pr43797-lcssa-for-multiple-outer-loop-blocks.ll
  LLVM :: Transforms/LoopInterchange/reductions-across-inner-and-outer-loop.ll
  LLVM :: Transforms/LoopInterchange/vector-gep-operand.ll



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

https://reviews.llvm.org/D118102