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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 3 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:106
+    }
+    Worklist.pushValue(&Old);
+  }
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110171