[PATCH] D64601: [MemorySSA] Use SetVector to avoid nondeterminism.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D64601#1583538 <https://reviews.llvm.org/D64601#1583538>, @asbirlea wrote:

> I don't know if a bot exists with LLVM_ENABLE_REVERSE_ITERATION set to true, but if it does, then the check I proposed for the uselist order would likely fail, no?


I believe there is (or some people with a vested interest in it - they might not be testing with an official buildbot). I think it's appropriate for you to validate that the test/change are justified by the reverse iteration mode before committing (by using a local build with reverse iteration).

>   ; CHECK: uselistorder i16 0, { 3, 2, 4, 1, 5, 0, 6 }
> 
> 
> 
> 
>> Does this test case exercise/justify all the container changes in this patch?
> 
> The test shows the need for an ordered container in `removeBlocks`. The patch changes all callsites for this method, and the sets it changes are also not used for much except the call to `removeBlocks`, so I think it should be fine.

Ah, makes sense - sorry for being vague/not looking more closely at the patch.

Does it seem like all call sites have the same ordering requirement? (or could some callers benefit from/get away with an unordered container?) - maybe that question's not worth trying to answer, not sure.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64601