[PATCH] D110171: [VectorCombine] Switch to using a worklist.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:106
+    }
+    Worklist.pushValue(&Old);
+  }
----------------
fhahn wrote:
> lebedev.ri wrote:
> > fhahn wrote:
> > > lebedev.ri wrote:
> > > > We also likely want to revisit users of the value of which we've just reduced the use-count.
> > > > But honestly this is quite fragile, because generally speaking we want to revisit **all** it's uses transitively.
> > > > https://bugs.llvm.org/show_bug.cgi?id=47238
> > > Do you think it's worth doing this in the current patch? Not sure if it is possible to come up with a test case with the current limited set of combines.
> > I don't know.
> > I'm simply saying that by not reiterating over the whole function afterwards,
> > we are very much likely missing transforms.
> Oh I see. I plan to add verification that re-running `VectorCombine` does not change the function as a follow-up. That should help surfacing such issues.
Yep, that would be good, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110171