[PATCH] D110173: [DebugInfo][InstrRef] Use correct (and existing) PHI placement utilities for machine locations

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
jmorse added reviewers: Orlando, StephenTozer, TWeaver.
Herald added a subscriber: hiraditya.
jmorse requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Over in D102158 <https://reviews.llvm.org/D102158> I outlined some flaws in my LiveDebugValues-rewrite, and pointed out that in reality it was SSA-construction with value propagation. I've been re-formulating the implementation to be defined in terms of SSA construction, which is what this patch does. This patch addresses the first "problem" that's solved, finding which value numbers are in which locations in the MachineFunction. The existing LLVM iterated-dominance-frontier tooling is used for PHI placement. This eliminates the lattice arrangement I was trying to work with before.

I find worked examples to be the best way of demonstrating how this works, thus this patch also adds a bunch of unit tests: different patterns of register definitions are tested over five or six weird control flow forms. They're all of the form:

- Set-up where the definitions (or copies) of registers are in the function,
- Run the PHI-placement / value-propagation function,
- Test that the correct values are found in the correct locations.

I can't promise that they exhaustively covers all input patterns, but they illustrate what this code is _supposed_ to do, and it'll help check future changes.

Over on the excellent compile-time-tracker pages, this patch [0] causes a roughly 1% slowdown across CTMark, possibly because my previous implementation wasn't doing PHI placement correctly, but it's quite likely that the IDF calculator is slower than what I was doing. I reckon that this performance can be recovered by only running the PHI placement algorithm for register units, then placing PHIs for all aliasing registers too. I haven't tried this yet though.

I have a similar patch coming that does this again, but for the other "problem", what values should variables have in each block. (Still writing unit tests for that).

[0] http://llvm-compile-time-tracker.com/compare.php?from=be06359638e470418d57c1c2592d15c4e468daf1&to=42e0868898a534d4772ee5e369ef29dfb89c95bc&stat=instructions


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110173

Files:
  llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
  llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
  llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp
  llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.h
  llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp
  llvm/unittests/CodeGen/InstrRefLDVTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110173.373896.patch
Type: text/x-patch
Size: 73644 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210921/9c977a2e/attachment.bin>