[PATCH] D58633: [InstCombine] remove one-use restriction for icmp+add constant fold

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 2 09:11:39 PST 2019


lebedev.ri added a comment.

In D58633#1765405 <https://reviews.llvm.org/D58633#1765405>, @spatel wrote:

> In D58633#1765212 <https://reviews.llvm.org/D58633#1765212>, @lebedev.ri wrote:
>
> > In D58633#1468821 <https://reviews.llvm.org/D58633#1468821>, @spatel wrote:
> >
> > > In D58633#1465428 <https://reviews.llvm.org/D58633#1465428>, @nikic wrote:
> > >
> > > > There's a similar one use check for the non-equality case in https://github.com/llvm-mirror/llvm/blob/fb1e04ff3c51a617c4944b2e05e8aa1b8c543e22/lib/Transforms/InstCombine/InstCombineCompares.cpp#L2389. This prevents the trivially true condition in https://rust.godbolt.org/z/qg0RjC from folding. That check was added (or rather moved) in rL341831 <https://reviews.llvm.org/rL341831>.
> > > >
> > > > Just wanted to add this as a datapoint for a case where dropping the restriction would be nice.
> > >
> >
> >
> > That should be resolved now, reverted in rG0f22e783a038 <https://reviews.llvm.org/rG0f22e783a038b6983f0fe161eef6cf2add3a4156>.
>
>
> Nice. Is that enough to resolve the motivating bug in the rust example?


I don't work on rust, but running that ir through -instcombine says yes.

> I think this patch is still held up until we figure out how to make LSR and/or SCEV reconstruct the original patterns that they are expecting.

Yes, i think so.


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

https://reviews.llvm.org/D58633





More information about the llvm-commits mailing list