[PATCH] D80236: [VectorCombine] position pass after SLP in the optimization pipeline rather than before

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat May 23 05:47:55 PDT 2020


spatel added a subscriber: dmgreen.
spatel added a comment.

In D80236#2051972 <https://reviews.llvm.org/D80236#2051972>, @nikic wrote:

> It turns out that the main impact of this change (both in terms of compile-time, and in terms of text size changes) is not the move of the VectorCombine pass, but the move of the EarlyCSE pass. If I leave EarlyCSE where it is and only move VectorCombine, the results is essentially noise <http://llvm-compile-time-tracker.com/compare.php?from=22ed724975d265086149dcac8d2c983c1e49f13f&to=96251186b9cd51566474e1e0645c9eb4b4deaf3e&stat=instructions>. (Which doesn't tell us anything about which EarlyCSE placement produces better code...)


I added the EarlyCSE cleanup as part of D75145 <https://reviews.llvm.org/D75145>. And it was noted there as causing perf regressions for ARM code (cc @dmgreen), so it was not ideal from the start.

I just commented out EarlyCSE locally from the pipeline, and we no longer need that on the motivating test from D75145 <https://reviews.llvm.org/D75145>. (Not exactly sure why - several things changed since then.)

But there is a regression with the new pass manager on the "add_aggregate_store" test that is changing here. Do you know why alias analysis is not working the same on that test for new and old PMs?

If we can fix that, then we can remove EarlyCSE from the pipeline with no immediate regressions (getting full test-suite/other results that agree with that would be ideal, but we could push the change and wait for fallout).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80236





More information about the llvm-commits mailing list