[PATCH] D64630: [DebugInfo] Address performance regression with r364515

Bjorn Pettersson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 30 10:51:44 PDT 2019


bjope added inline comments.


================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:1978
+// See comment for CheckDbgValuesInBlock; this is a specialised copy for
+// when one coalesced register is a physical register.
+static void CheckDbgValuesInBlockForPhysReg(const SlotIndexes &Slots,
----------------
aprantl wrote:
> ///
Not clear to me, just reading the description and the argument list, to understand if `Reg` is the physical register or the other register being coalesced.


================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:1982
+                                            const LiveInterval &RegLiveness,
+                                            unsigned Reg) {
+  SlotIndex BlockStart = Slots.getMBBStartIdx(MBB);
----------------
I think we want Reg to be of type Register here (at least in the future, but maybe we can get it right from the start).


================
Comment at: lib/CodeGen/RegisterCoalescer.cpp:2004
+    if (MI.isDebugValue() && !RegIsLive &&
+        MI.getOperand(0).isReg() && MI.getOperand(0).getReg() == Reg) {
+      // If so, point the DBG_VALUE to $noreg
----------------
Not sure if the old code cared much about it either, but I always wonder if we forget to consider sub registers when I see code that only looks at getReg() but not getSubReg() and not even mentions sub registers in any code comment. But since we are dealing with physical registers, then maybe getSubReg() is out-of-play here (although it is only one side of the coalescing pair that is physical).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64630





More information about the llvm-commits mailing list