[PATCH] D69584: [LDV][RAGreedy] Inform LiveDebugVariables about new VRegs added by InlineSpiller

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

Ouch @ this category of error, thanks for the patch.

Looks good with some comment nitpicks inline.



================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:275
+    }
+  }
+
----------------
Thanks for the refactor!


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:1097-1098
+  // locInts map. If an old vreg for example is spilled it might have been
+  // removed from LIS, but then we will keep the old vreg as location anyway
+  // until we rewrite locations later.
+  removeLocationIfUnused(OldLocNo);
----------------
It's unclear to me whether "then we will" is referring to the desired or undesired behaviour. To my mind the desired behaviour would be described:

  "If an old vreg for example is spilled it might have been removed from LIS, in which case we should drop the old vreg from the locations vector"

But I may have missed something.


================
Comment at: llvm/test/CodeGen/PowerPC/pr38899-split-register-at-spill.mir:17
+  attributes #0 = { nounwind }
+  attributes #1 = { nounwind readnone speculatable willreturn }
+
----------------
Best to drop attributes if we can.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69584