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

Pablo Barrio via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 11 01:54:42 PST 2016


pbarrio added a comment.

Sorry, forget what I said. I didn't get what you were trying to do at first (I've never used the "uses"). I will do the change, as this makes more clear that the operand we are interested in is the condition (i.e. operand 0).



================
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:
> pbarrio wrote:
> > 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.
> I'm not following your response... "I.uses()" and "I.users()" are basically the same thing, except that "I.uses()" also gives you operand numbers.
Sorry, forget what I said. I didn't get what you were trying to do at first (I've never used the "uses"). I will do the change tomorrow, as this makes more clear that the operand we are interested in is the condition (i.e. operand 0).


https://reviews.llvm.org/D26450





More information about the llvm-commits mailing list