[PATCH] D78586: [MachineVerifier] Add more checks for registers in live-in lists.

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 22 12:33:10 PDT 2020


efriedma marked an inline comment as done.
efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:2451
+        // We don't track liveness for reserved registers.
+        if (isReserved(LiveInReg))
+          continue;
----------------
ZhangKang wrote:
> In the file `lib/CodeGen/LivePhysRegs.cpp`:
> ```
>  57 /// Add uses to the set.
>  58 void LivePhysRegs::addUses(const MachineInstr &MI) {
>  59   for (const MachineOperand &MOP : phys_regs_and_masks(MI)) {
>  60     if (!MOP.isReg() || !MOP.readsReg())
>  61       continue;
>  62     addReg(MOP.getReg());
>  63   }
>  64 }
>  65
>  66 /// Simulates liveness when stepping backwards over an instruction(bundle):
>  67 /// Remove Defs, add uses. This is the recommended way of calculating liveness.
>  68 void LivePhysRegs::stepBackward(const MachineInstr &MI) {
>  69   // Remove defined registers and regmask kills from the set.
>  70   removeDefs(MI);
>  71
>  72   // Add uses to the set.
>  73   addUses(MI);
>  74 }
> ```
> 
> When we calculate the liveness by using the method `stepBackend`, we have added the reserved registers into the liveness in `addUses`, but here, we don't track the reserved registers. Are the `addUses` and the verification correct?
If you look at other APIs, like available(), it intentionally ignores reserved registers.  Maybe we should try to do something more complete, but in general it doesn't make sense to track liveness for such registers. (For example, the stack pointer has external constraints on what it can contain, which can't be understood using normal liveness rules.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78586





More information about the llvm-commits mailing list