[PATCH] D64884: [PHINode] Preserve use-list order when removing incoming values.

Mon Sep 16 13:18:43 PDT 2019

fhahn marked an inline comment as done.
fhahn added a comment.

In D64884#1661810 <https://reviews.llvm.org/D64884#1661810>, @efriedma wrote:

> > My motivation for this change is to fix a case where we end up with a non-deterministic use-list order after simplifycfg, which I think is caused by this trashing of the use-lists.
>
> I don't see how this could be a problem. Sure, mixing up the order is sort of confusing, but it should get mixed up in a deterministic way.

I think I came across this for a codegen non-determinism case fixed by rL365970 <https://reviews.llvm.org/rL365970>.

The issue there was that removing blocks in non-deterministic order caused non-deterministic codegen later on, because depending on the deletion order, the use lists order of some PHIs were different. With this fix, we would not have to enforce an order on block deletions, at least not because of the use-list issue.

================
Comment at: llvm/lib/IR/Instructions.cpp:129
-  // clients might not expect this to happen.  The code as it is thrashes the
-  // use/def lists, which is kinda lame.
-  std::copy(op_begin() + Idx + 1, op_end(), op_begin() + Idx);
----------------
efriedma wrote:
> Even if this patch fixes the use/def list thrashing, swap would still be faster.
Yep, I also fixed a similar use-list issue when wapping, so we could swap here as well (D64886).

But that would make the order of incoming values dependent on the deletion order, I think, defeating the original motivation of the patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64884