[PATCH] D103514: [LV] Support sinking recipe in replicate region after another region.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 4 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9226
+        continue;
+      }
+    }
----------------
Ayal wrote:
> Handle the "SinkRegion && !TargetRegion" case here, splitting TargetBlock etc., instead of last below?
Done, thanks!


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9228
+    }
+    if (TargetRegion) {
+      // The target is in a replication region, make sure to move Sink to
----------------
Ayal wrote:
> Handle this relatively simple "!SinkRegion && TargetRegion" case 2nd, after the simplest !SinkRegion && !TargetRegion? (I.e., can check first if (!SinkRegion), etc.)
I restructured the code, it should be a bit more compact now.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9200
+      assert(Target->getParent()->size() == 1 &&
+             "Target must be in a replicator with a single recipe");
+
----------------
Ayal wrote:
> fhahn wrote:
> > Ayal wrote:
> > > These asserts are better placed inside the lambda?
> > I moved the SESE one. The other one would be a bit more work, because currently the lambda does not have access to Target's/Sink's parents. A general helper to check for replicate regions in the future may be helpful.
> Lambda can check if R->getParent() has size 1, right? This should be asserted for both sink and target.
Yes, I moved the assertion,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103514