[PATCH] D86530: [InstCombine] PHI-of-extractvalues -> extractvalue-of-PHI, aka invokes are bad

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

@spatel

Ok, so

> There are two reasons why:
>
> 1. After this fold, as it can be seen in tests, we may (will) end up with trivially redundant PHI nodes. We don't CSE them in InstCombine presently, which means that EarlyCSE needs to run and then InstCombine rerun.
> 2. But then, EarlyCSE not only manages to fold such redundant PHI's, it also sees that the extract-insert chain recreates the original aggregate, and replaces it with the original aggregate.
>
> The take-aways are
>
> 1. We maybe should do most trivial, same-BB PHI CSE in InstCombine
> 2. I need to check if what other patterns remain, and how they can be resolved. (i.e. i wonder if `foldAggregateConstructionIntoAggregateReuse()` might go away)

I looked at remaining interesting patterns, and i //believe// this sinking won't be able to deal with everything.
there may be genuine extra uses of extracts, namely `__cxa_begin_catch()`.

So before i look into `foldAggregateConstructionIntoAggregateReuse()` further,
what are your thoughts about teaching InstSimplify to do most basic CSE for PHI nodes?
(purely by comparing that the incoming values match)

This will ease some phase ordering issues, since then we won't need to run earlycse and rerun instcombine.

Effect on vanilla test-suite + RawSpeed:

  | statistic name                                     | baseline  | proposed  |      Δ |        % |    \|%\| |
  |----------------------------------------------------|-----------|-----------|-------:|---------:|---------:|
  | asm-printer.EmittedInsts                           | 7942505   | 7942558   |     53 |    0.00% |    0.00% |
  | assembler.ObjectBytes                              | 273069800 | 273085376 |  15576 |    0.01% |    0.01% |
  | correlated-value-propagation.NumPhis               | 18825     | 18958     |    133 |    0.71% |    0.71% |
  | early-cse.NumCSE                                   | 2183398   | 2183342   |    -56 |    0.00% |    0.00% |
  | early-cse.NumCSELoad                               | 317796    | 317801    |      5 |    0.00% |    0.00% |
  | early-cse.NumSimplify                              | 550017    | 542090    |  -7927 |   -1.44% |    1.44% |
  | instcombine.NumAggregateReconstructionsSimplified  | 108       | 4502      |   4394 | 4068.52% | 4068.52% |
  | instcombine.NumCombined                            | 3635533   | 3659879   |  24346 |    0.67% |    0.67% |
  | instcombine.NumDeadInst                            | 1765202   | 1770173   |   4971 |    0.28% |    0.28% |
  | instcombine.NumPHIsOfExtractValues                 | 37546     | 37521     |    -25 |   -0.07% |    0.07% |
  | instcount.NumBrInst                                | 871857    | 871838    |    -19 |    0.00% |    0.00% |
  | instcount.NumCallInst                              | 1758402   | 1758818   |    416 |    0.02% |    0.02% |
  | instcount.NumExtractValueInst                      | 11483     | 11477     |     -6 |   -0.05% |    0.05% |
  | instcount.NumInsertValueInst                       | 580       | 578       |     -2 |   -0.34% |    0.34% |
  | instcount.NumInvokeInst                            | 59478     | 59502     |     24 |    0.04% |    0.04% |
  | instcount.NumLandingPadInst                        | 34215     | 34214     |     -1 |    0.00% |    0.00% |
  | instcount.NumPHIInst                               | 331116    | 331086    |    -30 |   -0.01% |    0.01% |
  | instcount.NumResumeInst                            | 8062      | 8061      |     -1 |   -0.01% |    0.01% |
  | instcount.NumRetInst                               | 100772    | 100770    |     -2 |    0.00% |    0.00% |
  | instcount.TotalBlocks                              | 1077166   | 1077168   |      2 |    0.00% |    0.00% |
  | instcount.TotalFuncs                               | 101442    | 101441    |     -1 |    0.00% |    0.00% |
  | instcount.TotalInsts                               | 8833575   | 8833896   |    321 |    0.00% |    0.00% |
  | simplifycfg.NumInvokes                             | 4298      | 4406      |    108 |    2.51% |    2.51% |
  | simplifycfg.NumSimpl                               | 1018189   | 998050    | -20139 |   -1.98% |    1.98% |

... so it again results in improvements in `invoke`->`call` fold.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86530