[PATCH] D87154: [WIP][Statepoints] Change statepoint machine instr format to better suit VReg lowering.

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/StackMaps.h:228
+  unsigned
+  getGCPointerMap(SmallVectorImpl<std::pair<unsigned, unsigned>> &GCMap);
+
----------------
doc


================
Comment at: llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp:372
   bool findRegistersToSpill() {
+    SmallSet<Register, 8> GCRegs;
+    for (const auto &Def : MI.defs())
----------------
Add a comment that it is guaranteed (by what?) that all uses with reg has tied def.
May be add and assert function?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp:611
 
-  for (unsigned i = 0; i < SI.Bases.size(); ++i) {
-    SDValue SDV = Builder.getValue(SI.Bases[i]);
-    if (AlwaysSpillBase || !LowerAsVReg.count(SDV))
-      reservePreviousStackSlotForValue(SI.Bases[i], Builder);
-    SDV = Builder.getValue(SI.Ptrs[i]);
+  for (const Value *V : SI.Ptrs) {
+    SDValue SDV = Builder.getValue(V);
----------------
why not to iterate over LoweredGCPtrs?


================
Comment at: llvm/lib/CodeGen/StackMaps.cpp:94
 
+int StatepointOpers::getFirstGCPtrIdx() {
+  unsigned NumDeoptsIdx = getNumDeoptArgsIdx();
----------------
To simplify code review you could move this function here as NFC first...


================
Comment at: llvm/lib/CodeGen/StackMaps.cpp:395
   // Parse operands.
-  while (MOI != MOE) {
-    MOI = parseOperand(MOI, MOE, Locations, LiveOuts);
+  if (MI.getOpcode() == TargetOpcode::STATEPOINT) {
+    LLVM_DEBUG(dbgs() << "record statepoint : " << MI << "\n");
----------------
separate to utility function?


================
Comment at: llvm/lib/CodeGen/StackMaps.cpp:423
+    if (NumGCPointers) {
+      // Map logical index of GC ptr to MI operand index.
+      SmallVector<unsigned, 8> GCPtrIndices;
----------------
separate to utility function?


================
Comment at: llvm/test/CodeGen/X86/statepoint-vreg.mir:3
 # RUN: llc -o - %s -fixup-allow-gcptr-in-csr=true -start-after=finalize-isel | FileCheck %s
+# XFAIL:*
 
----------------
Why it should fail now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87154