[PATCH] D26450: [JumpThreading] Prevent non-deterministic use lists

[Pablo Barrio via llvm-commits]

pbarrio added a comment.

I have investigated the -preserve-ll-uselistorder option. It sets option "ShouldPreserveUseListOrder" to true in the module printer. According to the documentation:

"If ShouldPreserveUseListOrder, then include uselistorder directives so that use-lists can be recreated when reading the assembly. "

This will make sure that the order is the same across different calls to opt or across different tools, and I *think* it is now enabled by default. I could call opt twice with jump threading and compare the resulting testlists, but this is a fragile test as it may pass even when there is a bug. Something like this would be a very generic test:

; RUN: opt < %s -jump-threading > %t1
; RUN: opt < %s -jump-threading > %t2
; RUN: diff %t1 %t2

But I don't think it adds much value, so I haven't updated the patch.



================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:2048
         if (auto *SI = dyn_cast<SelectInst>(U))
-          Selects.push_back(SI);
+          Selects.insert(SI);
       if (Selects.size() <= 1)
----------------
efriedma wrote:
> Looking at this a bit closer, there's another way you could write this: you can check whether the use is the condition of a select.  Something like:
> 
>     for (Use& U : I.uses()) {
>       auto *SI = dyn_cast<SelectInst>(U.getUser());
>       if (SI && U.getOperandNo() == 0)
>         Selects.push_back(SI);
>     }
> 
> That naturally avoids duplicates, and is probably a bit closer to your original intent.
That was the first approach I tried, but it will unfold selects in cases where it is not necessary. I only want to unfold when there are two or more selects that use the same condition, so I need to check the condition's users instead of the select's uses.


https://reviews.llvm.org/D26450