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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

In D110171#3013095 <https://reviews.llvm.org/D110171#3013095>, @fhahn wrote:

> Remove DbgPrefix, use pushValue, avoid using deferred worklist part.
>
> In D110171#3013003 <https://reviews.llvm.org/D110171#3013003>, @nikic wrote:
>
>>> This may either be an oversight, or an intentional improvement that guarantees that in this pass we don't repeat instcombine's mistake
>>> of relying on 3., but consistently add everything we want reprocessed into worklist.
>
> Thanks for elaborating! I think for now it is not worth re-processing the whole function, because the combines are very limited in scope. It would probably be best to make sure the correct instructions are added to the worklist instead, e.g. note the updates around line 522 which ensure we can clean up now-dead extracts.

Aha, just checking. Sounds good to me.
Seems fine, but probably wait for @spatel.



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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110171