[all-commits] [llvm/llvm-project] 607f8c: [AMDGPU]: Fix failing assertion in SIMachineScheduler

[Jannik Silvanus via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 607f8ced39259b672f4141190f4e7e5ab3375cd1
      https://github.com/llvm/llvm-project/commit/607f8ced39259b672f4141190f4e7e5ab3375cd1
  Author: Jannik Silvanus <jannik.silvanus at amd.com>
  Date:   2022-04-21 (Thu, 21 Apr 2022)

  Changed paths:
    M llvm/lib/Target/AMDGPU/SIMachineScheduler.cpp
    A llvm/test/CodeGen/AMDGPU/si-scheduler-exports.ll

  Log Message:
  -----------
  [AMDGPU]: Fix failing assertion in SIMachineScheduler

This fixes the assertion failure "Loop in the Block Graph!".

SIMachineScheduler groups instructions into blocks (also referred to
as coloring or groups) and then performs a two-level scheduling:
inter-block scheduling, and intra-block scheduling.

This approach requires that the dependency graph on the blocks which
is obtained by contracting the blocks in the original dependency graph
is acyclic. In other words: Whenever A and B end up in the same block,
all vertices on a path from A to B must be in the same block.

When compiling an example consisting of an export followed by
a buffer store, we see a dependency between these two. This dependency
may be false, but that is a different issue.
This dependency was not correctly accounted for by SiMachineScheduler.

A new test case si-scheduler-exports.ll demonstrating this is
also added in this commit.

The problematic part of SiMachineScheduler was a post-optimization of
the block assignment that tried to group all export instructions into
a separate export block for better execution performance. This routine
correctly checked that any paths from exports to exports did not
contain any non-exports, but not vice-versa: In case of an export with
a non-export successor dependency, that single export was moved
to a separate block, which could then be both a successor and a
predecessor block of a non-export block.

As fix, we now skip export grouping if there are exports with direct
non-export successor dependencies. This fixes the issue at hand,
but is slightly pessimistic:
We *could* group all exports into a separate block that have neither
direct nor indirect export successor dependencies.
We will review the potential performance impact and potentially
revisit with a more sophisticated implementation.

Note that just grouping all exports without direct non-export successor
dependencies could still lead to illegal blocks, since non-export A
could depend on export B that depends on export C. In that case,
export C has no non-export successor, but still may not be grouped
into an export block.