[PATCH] D71093: [InstCombine] Insert instructions before adding them to worklist

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D71093#1773043 <https://reviews.llvm.org/D71093#1773043>, @lebedev.ri wrote:

> In D71093#1773021 <https://reviews.llvm.org/D71093#1773021>, @spatel wrote:
>
> > In D71093#1772839 <https://reviews.llvm.org/D71093#1772839>, @lebedev.ri wrote:
> >
> > > I'm not convinced this is an improvement overall.
> > >
> > > As a concrete point: we will no longer print the 'new' if said 'new' isn't a new instruction;
> > >  as in, printing of new will now depend on the fact that it will be added into worklist,
> > >  which won't be the case if we returned preexisting instruction.
> >
> >
> > If we return the existing instruction, don't we always fall down to the else at line 3372 and print it there?
>
>
> No, because we only get there if we return *the same* instruction we just visited.
>  I'm talking about the case like:
>
>   %x = mul i8 %y, 42
>   %t0 = sub i8 0, %x
>   %t1 = sub i8 0, %t0
>  
>   visit %t1
>   ... returned %x
>   %x != %t1
>


Ah, disregard my suggestion then. Sorry for the noise. I don't have strong feelings about the debug spew, so feel free to proceed with whatever the consensus leads to.


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

https://reviews.llvm.org/D71093