[PATCH] D137159: [LoopVectorize] Introduce CanSinkMemoryInstrs to isFixedOrderRecurrence

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a subscriber: gilr.
fhahn added a comment.

Thanks for sharing the patch! I had a look at some of the patches I had around from a while ago and from that I think there are a couple of potential issues, that make addressing this as extension to current `isFixedOrderRecurrence` difficult:

- Sink candidates could be stores that are part of an interleave group, so even if we sink the original store, the actual interleave group may be created earlier and end up in an incorrect position. We don't know about interleave groups in `isFixedOrderRecurrence`.
- Sink candidates may move past other memory instructions that may alias them, which I think the current patch doesn't check for.

The current logic of trying to collect a sink map and ensure no constraints are violated later on is quite fragile (there have been several issues with violations in the past) and further extending the logic doesn't seem ideal. One option discussed with @Ayal & @gilr a while ago was to move fixing up re-ordering the instructions to VPlan. At this point we can simplify re-order according to the constraints and bail out if no order can be found. One blocker for this I think is that there's no way to query dominance across regions in VPlan at the moment. I am working on fixing that and hopefully will be able to share a prototype for this approach in a few weeks.

(AFAIK MemoryDependenceAnalysis is considered deprecated and we are working on removing it (most users have already been removed, including DSE & MemCpyOpt).)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137159