[PATCH] D111627: [DebugInfo][InstrRef] Recover some performance by calculating IDF for register units instead of all their alises

Tue Oct 12 04:47:58 PDT 2021

jmorse added a comment.

> Out of interest do you have any numbers for the positive performance impact of this patch?

Loss from PHI installation:
http://llvm-compile-time-tracker.com/compare.php?from=9b9e6f1d4a7b15d5a4a076cd7e9dfbe3cc9afce6&to=c039d62f7f2bbea4cca09be7d2355700e972f9e6&stat=instructions

Gain from this patch:
http://llvm-compile-time-tracker.com/compare.php?from=7ee343ecae52ea3cd843e508e322b6b10c649a56&to=bc07cf76a31fa769786d6631fe25586bd1396c34&stat=instructions

They're not directly comparable numbers because inbetween is the variable-location PHI installation patch, which adds additional performance loss. The full set of patches are here under the "use-ssa-phi-placement3" branch, unfortunately the commit messages are written in Jeremy-speak so might not be intelligible. The order goes:

- Turn on instr-ref
- Install 2 PHI placement patches
- This patch
- Another vloc performance patch
- 2 attempts to improve performance further with corresponding reverts
- A final vloc performance patch that actually works,
- Another patch that didn't deliver any gains.

https://llvm-compile-time-tracker.com/?config=NewPM-ReleaseLTO-g&stat=instructions&remote=jmorse

================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:1786
+    if (AnyIllegal) {
+      SlotsToPHI.insert(L); // WAS: put reg in regunitstophiup directly
+      continue;
----------------
Orlando wrote:
> nits: If you're also stuffing (arg) registers in too, maybe `SlotsToPHI` should be renamed. Something like `LocsToPHI` or `LocsWithoutRegUnits`?
> Also is this a dev comment that leaked into the patch?
yes and yes

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111627