[PATCH] D112133: [DebugInfo][Instr] Track subregisters across stack spills/restores

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando accepted this revision.
Orlando added a comment.
This revision is now accepted and ready to land.

Ok, been over this again and LGTM (for real this time). I'm a big fan of all the testing.



================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h:458
+    assert(ID >= NumRegs);
+    ID -= NumRegs;
+    ID /= NumSlotIdxes;
----------------
jmorse wrote:
> Orlando wrote:
> > I am a little confused with the large number of different indices flying around in here, so this might be a silly question. Should there be another number subtracted here, equivalent to the `Idx` added to the ID in the function above (`getSpillIDWithIdx`)? Or is this auto-magically handled due to integer division rounding or something?
> The latter; I'm relying on the fact that integer division truncates any remainder, the "index" / "idx" / whatever will be eliminated from "ID" in the process. To illustrate, here's an example LocIDToLocIdx table, assuming there are 7 "Idxes"
> 
>     0 => AL
>     1 => AH
>     [... some ~288 X86 registers]
>     288 => Slot 0 Idx 0
>     289 => Slot 0 Idx 1
>     290 => Slot 0 Idx 2
>     291 => Slot 0 Idx 3
>     292 => Slot 0 Idx 4
>     293 => Slot 0 Idx 5
>     294 => Slot 0 Idx 6
>     295 => Slot 1 Idx 0
>     296 => Slot 1 Idx 1
>     297 => Slot 1 Idx 2
>     298 => Slot 1 Idx 3
>     299 => Slot 1 Idx 4
>     300 => Slot 1 Idx 5
>     301 => Slot 1 Idx 6
> 
> This function subtracts to find the range where the "slots" are recorded, then divides by 7 to remove the "Idx" part, leaving only the slot number.
SGTM, ty for that. Might be worth just mentioning that in a comment briefly but I'll leave that up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112133