[PATCH] D71448: [Legalizer] Making artifact combining order-independent

[Roman Tereshin via Phabricator via llvm-commits]

rtereshin created this revision.
rtereshin added reviewers: volkan, aditya_nandakumar, qcolombet, paquette, aemerson, dsanders.
Herald added subscribers: llvm-commits, Petar.Avramovic, atanasyan, jrtc27, JDevlieghere, hiraditya, nhaehnle, jvesely, sdardis.
Herald added a project: LLVM.

Legalization algorithm is complicated by two facts:

1. While regular instructions should be possible to legalize in an isolated, per-instruction, context-free manner, legalization artifacts can only be eliminated in pairs, which could be deeply, and ultimately arbitrary nested: { [ () ] }, where which paranthesis kind depicts an artifact kind, like extend, unmerge, etc. Such structure can only be fully eliminated by simple local combines if they are attempted in a particular order (inside out), or alternatively by repeated scans each eliminating only one innermost pair, resulting in O(n^2) complexity.
2. Some artifacts might in fact be regular instructions that could (and sometimes should) be legalized by the target-specific rules. Which means failure to eliminate all artifacts on the first iteration is not a failure, they need to be tried as instructions, which may produce more artifacts, including the ones that are in fact regular instructions, resulting in a non-constant number of iterations required to finish the process.

I trust the recently introduced termination condition (no new artifacts
were created during as-a-regular-instruction-retrial of artifacts not
eliminated on the previous iteration) to be efficient in providing
termination, but only performing the legalization in full if and only if
at each step such chains of artifacts are successfully eliminated in
full as well.

Which is currently not guaranteed, as the artifact combines are applied
only once and in an arbitrary order that has to do with the order of
creation or insertion of artifacts into their worklist, which is a no
particular order.

In this patch I make a small change to the artifact combiner, making it
to re-insert into the worklist immediate (modulo a look-through copies)
artifact users of each vreg that changes its definition due to an
artifact combine.

Here the first scan through the artifacts worklist, while not
being done in any guaranteed order, only needs to find the innermost
pair(s) of artifacts that could be immediately combined out. After that
the process follows def-use chains, making them shorter at each step, thus
combining everything that can be combined in O(n) time.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71448

Files:
  llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
  llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
  llvm/lib/Target/Mips/MipsRegisterBankInfo.cpp
  llvm/test/CodeGen/AArch64/GlobalISel/legalize-merge-values.mir
  llvm/test/CodeGen/AArch64/GlobalISel/legalizer-combiner-zext-trunc-crash.mir
  llvm/test/CodeGen/AArch64/GlobalISel/retry-artifact-combine.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/artifact-combiner-sext.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-fcopysign.mir
  llvm/unittests/CodeGen/GlobalISel/LegalizerTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71448.233723.patch
Type: text/x-patch
Size: 39837 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191213/bfd566c4/attachment.bin>