[PATCH] D99284: [RegAllocFast] properly handle STATEPOINT instruction.

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added inline comments.


================
Comment at: llvm/lib/CodeGen/RegAllocFast.cpp:164
+    bool isClobberedByRegMasks(MCPhysReg PhysReg) const {
+      for (const auto *RM : RegMasks)
+        if (MachineOperand::clobbersPhysReg(RM, PhysReg))
----------------
any_of?


================
Comment at: llvm/lib/CodeGen/RegAllocFast.cpp:1288
+    for (auto R : AssignedTiedDefs) {
+      assert(!isClobberedByRegMasks(R) &&
+             "tied def allocated to clobbered register");
----------------
all_of?


================
Comment at: llvm/lib/CodeGen/RegAllocFast.cpp:1141
+  // Set of registers allocated to tied defs.
+  SmallSet<Register, 16> AssignedTiedDefs;
+
----------------
skatkov wrote:
> you do not need to collect all tied-defs assigned. As soon as it is under an assert, you can just iterate over defs of MI and check that if it is def it is not assigned to clobbered register.
> Moreover you can move an assert to end of an allocation and also check that tied-use is allocated to the same register.
> 
> If you decide to leave it in this way - please ensure that this declaration and inserting in the set is not used in product mode.
> you do not need to collect all tied-defs assigned. As soon as it is under an assert, you can just iterate over defs of MI and check that if it is def it is not assigned to clobbered register.
> Moreover you can move an assert to end of an allocation and also check that tied-use is allocated to the same register.
> 
> If you decide to leave it in this way - please ensure that this declaration and inserting in the set is not used in product mode.

Did not get any answer on this comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99284