[PATCH] D118642: [IVDescriptor] Find original 'Previous' for first-order recurrences.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

In D118642#3361875 <https://reviews.llvm.org/D118642#3361875>, @fhahn wrote:

> In D118642#3361598 <https://reviews.llvm.org/D118642#3361598>, @dyung wrote:
>
>> @fhahn This change seems to be hitting an assertion failure in one of our internal tests. I have filed GHI#54223 <https://github.com/llvm/llvm-project/issues/54223> with a repro. Can you take a look?
>
> Thanks, that should be fixed by de8ac485e5b7 <https://reviews.llvm.org/rGde8ac485e5b76cc48c396a0748499277690d1012>
>
> The issue was that the order in the map vector wasn't correct. That has been fixed by remove the entry before re-sinking. This ensures it will be sunk after all earlier instructions have been sunk.

Good catch!
Would be good to document the order: "if x and y both belong to SinkAfter and SinkAfter[x]==y, then y must appear before x"
This is enforced by always inserting at the end, and making sure to insert sinking chains in this order.
Would be good to assert that this order is maintained, either at each insertion, or prior to execution?
Wonder if alternatively SinkAfter could be an std::map with an explicit key comparison function, similar to InstrsToSink(?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118642