[all-commits] [llvm/llvm-project] 4791cb: [NFC][InstCombine] Add more tests for aggregate re...

[Roman Lebedev via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 4791cbdaf9e2ed420d61291439f1154477a2b2a2
      https://github.com/llvm/llvm-project/commit/4791cbdaf9e2ed420d61291439f1154477a2b2a2
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2020-08-18 (Tue, 18 Aug 2020)

  Changed paths:
    M llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll

  Log Message:
  -----------
  [NFC][InstCombine] Add more tests for aggregate reconstruction w/ PHI handling

Even without handling several layers of PHI nodes,
we can handle more cases, as `@test6` shows.


  Commit: 4973ca3eac998068cebe7bd23f47876e7c92b87f
      https://github.com/llvm/llvm-project/commit/4973ca3eac998068cebe7bd23f47876e7c92b87f
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2020-08-18 (Tue, 18 Aug 2020)

  Changed paths:
    M llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
    M llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll

  Log Message:
  -----------
  [NFC][InstCombine] PHI-aware aggregate reconstruction: insert PHI node manually

This is NFC at the moment, because right now we always insert the PHI
into the same basic block in which the original `insertvalue` instruction
is, but that will change.

Also, fixes addition of the suffix to the value names.


  Commit: f4f673e0e36937954c2410b2dfd5ca8e39ccffa5
      https://github.com/llvm/llvm-project/commit/f4f673e0e36937954c2410b2dfd5ca8e39ccffa5
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2020-08-18 (Tue, 18 Aug 2020)

  Changed paths:
    M llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp

  Log Message:
  -----------
  [NFC][InstCombine] PHI-aware aggregate reconstruction: don't capture UseBB in lambdas, take it as argument

In a following patch, UseBB will be detected later,
so capturing it is potentially error-prone (capture by ref vs by val).

Also, parametrized UseBB will likely be needed
for multiple levels of PHI indirections later on anyways.


  Commit: 03127f795b8244c1039c18d4391374707a3dc75e
      https://github.com/llvm/llvm-project/commit/03127f795b8244c1039c18d4391374707a3dc75e
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2020-08-18 (Tue, 18 Aug 2020)

  Changed paths:
    M llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
    M llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll

  Log Message:
  -----------
  [InstCombine] PHI-aware aggregate reconstruction: correctly detect "use" basic block

While the original implementation added in D85787 / ae7f08812e0995481eb345cecc5dd4529829ba44
is not incorrect, it is known to be suboptimal.

In particular, it is not incorrect to use the basic block
in which the original `insertvalue` instruction is located
as the merge point, that is not necessarily optimal,
as `@test6` shows.

We should look at all the AggElts, and, if they are all defined
in the same basic block, then that is the basic block we should use.

On RawSpeed library, this catches +4% (+50) more cases.
On vanilla LLVM test-suits, this catches +12% (+92) more cases.


Compare: https://github.com/llvm/llvm-project/compare/a1a3b86910e4...03127f795b82