[PATCH] D111300: [VPlan] Keep induction recipes in header.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

In D111300#3120991 <https://reviews.llvm.org/D111300#3120991>, @fhahn wrote:

> In D111300#3120415 <https://reviews.llvm.org/D111300#3120415>, @dyung wrote:
>
>> Hi, this change is hitting an assertion failure when run on one of our internal tests. I have put the details in PR52460 <https://bugs.llvm.org/show_bug.cgi?id=52460>, can you take a look?
>
> Thanks for the report!
>
> The issue is that sink-after for first-order recurrences might need to sink after the original truncate instruction of an optimized IV recipe, so we need to keep them in the original position until *after* sink-after. I pushed e7f1232cb777 <https://reviews.llvm.org/rGe7f1232cb777ac7408c7b4b7f5c421986bad3a82> to fix the crash. @Ayal it would be great if you could take a post-commit look, in case you have any ideas/thoughts how to resolve this differently.

An alternative fix: hoist trunc's into header phi recipes when created, as this patch originally did, and ignore/silence any sink-after targeting such truncs. Sink-afters ensure that uses of a FOR phi which originally appear before the Previous feeding the FOR phi, will appear after it; if Previous is hoisted to become a header phi, any such use is sure to appear after it, w/o needing to sink. I.e., the hoisting cancels the sinking. Sinking a user after a hoisted Previous may actually hoist the user, thereby potentially breaking dependences, as demonstrated by the PR/testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111300