[PATCH] D105428: [X86] Correctly return src/dest register from stack spill/restore recogniser hooks

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
jmorse added reviewers: craig.topper, spatel, RKSimon, andreadb.
Herald added subscribers: pengfei, hiraditya.
jmorse requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

LLVM provides target hooks to recognise stack spill and restore instructions, such as isLoadFromStackSlot, and it also provides post frame elimination versions such as isLoadFromStackSlotPostFE. These are supposed to return the store-source and load-destination registers; unfortunately on X86, the PostFE recognisers just return "1", apparently to signify "yes it's a spill/load". This patch alters the hooks to correctly return the store-source and load-destination registers:

- The store source is the operand at position X86::AddrNumOperands -- so the first register after the memory operands. This is the same behaviour as pre frame elimination isStoreToStackSlot.
- The load destination register is at operand zero, in line with existing LLVM conventions.

This is really useful for debug-info (see child revision) as we it helps follow variable values as they move on/off the stack. There should be no codegen changes: the only other users of these PostFE target hooks are MachineInstr::getRestoreSize and MachineInstr::getSpillSize, which don't attempt to interpret the returned register location.

When these hooks were originally added [0] they came with a rider that they use a heuristic and so aren't reliable for correctness, which makes me cautious. However, I think this refers to the fact that the list of spill / restore instructions isn't necessarily complete, rather than the determination of the register location can't be guaranteed.

There's no test for this patch: because nothing codegen related actually uses this information. Instead, it's tested by a debug-info test, which I'll upload shortly and add as a child revision. (I figured it'd be better to not mix debug-info and X86 patches).

[0] https://github.com/jmorse/llvm-project/commit/2f4c37425b5ce7809601f0dbe5e45c9c0b17a17e


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105428

Files:
  llvm/lib/Target/X86/X86InstrInfo.cpp


Index: llvm/lib/Target/X86/X86InstrInfo.cpp
===================================================================
--- llvm/lib/Target/X86/X86InstrInfo.cpp
+++ llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -905,7 +905,7 @@
       FrameIndex =
           cast<FixedStackPseudoSourceValue>(Accesses.front()->getPseudoValue())
               ->getFrameIndex();
-      return 1;
+      return MI.getOperand(0).getReg();
     }
   }
   return 0;
@@ -940,7 +940,7 @@
       FrameIndex =
           cast<FixedStackPseudoSourceValue>(Accesses.front()->getPseudoValue())
               ->getFrameIndex();
-      return 1;
+      return MI.getOperand(X86::AddrNumOperands).getReg();
     }
   }
   return 0;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D105428.356478.patch
Type: text/x-patch
Size: 689 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210705/90bed81e/attachment.bin>