[Statepoint] Add getters to StatepointOpers.
authorDenis Antrushin <dantrushin@gmail.com>
Tue, 14 Apr 2020 15:30:51 +0000 (18:30 +0300)
committerDenis Antrushin <dantrushin@gmail.com>
Wed, 15 Apr 2020 11:31:42 +0000 (14:31 +0300)
To simplify future work on statepoint representation, hide
direct access to statepoint field indices and provide getters
for them. Add getters for couple more statepoint fields.

This also fixes two bugs in MachineVerifier for statepoint:
First, the `break` statement was falling out of `if` statement
scope, thus disabling following checks.
Second, it was incorrectly accessing some fields like CallingConv -
StatepointOpers gives index to their value directly, not to
preceeding field type encoding.

Reviewed By: skatkov
Differential Revision: https://reviews.llvm.org/D78119

llvm/include/llvm/CodeGen/StackMaps.h
llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
llvm/lib/CodeGen/MachineVerifier.cpp

index 63547e5..e33ee22 100644 (file)
@@ -156,23 +156,44 @@ class StatepointOpers {
   // Flags should be part of meta operands, with args and deopt operands, and
   // gc operands all prefixed by their length and a type code. This would be
   // much more consistent.
-public:
-  // These values are aboolute offsets into the operands of the statepoint
+
+  // These values are absolute offsets into the operands of the statepoint
   // instruction.
   enum { IDPos, NBytesPos, NCallArgsPos, CallTargetPos, MetaEnd };
 
-  // These values are relative offests from the start of the statepoint meta
+  // These values are relative offsets from the start of the statepoint meta
   // arguments (i.e. the end of the call arguments).
   enum { CCOffset = 1, FlagsOffset = 3, NumDeoptOperandsOffset = 5 };
 
+public:
   explicit StatepointOpers(const MachineInstr *MI) : MI(MI) {}
 
+  /// Get index of statepoint ID operand.
+  unsigned getIDPos() const { return IDPos; }
+
+  /// Get index of Num Patch Bytes operand.
+  unsigned getNBytesPos() const { return NBytesPos; }
+
+  /// Get index of Num Call Arguments operand.
+  unsigned getNCallArgsPos() const { return NCallArgsPos; }
+
   /// Get starting index of non call related arguments
   /// (calling convention, statepoint flags, vm state and gc state).
   unsigned getVarIdx() const {
     return MI->getOperand(NCallArgsPos).getImm() + MetaEnd;
   }
 
+  /// Get index of Calling Convention operand.
+  unsigned getCCIdx() const { return getVarIdx() + CCOffset; }
+
+  /// Get index of Flags operand.
+  unsigned getFlagsIdx() const { return getVarIdx() + FlagsOffset; }
+
+  /// Get index of Number Deopt Arguments operand.
+  unsigned getNumDeoptArgsIdx() const {
+    return getVarIdx() + NumDeoptOperandsOffset;
+  }
+
   /// Return the ID for the given statepoint.
   uint64_t getID() const { return MI->getOperand(IDPos).getImm(); }
 
@@ -181,11 +202,19 @@ public:
     return MI->getOperand(NBytesPos).getImm();
   }
 
-  /// Returns the target of the underlying call.
+  /// Return the target of the underlying call.
   const MachineOperand &getCallTarget() const {
     return MI->getOperand(CallTargetPos);
   }
 
+  /// Return the calling convention.
+  CallingConv::ID getCallingConv() const {
+    return MI->getOperand(getCCIdx()).getImm();
+  }
+
+  /// Return the statepoint flags.
+  uint64_t getFlags() const { return MI->getOperand(getFlagsIdx()).getImm(); }
+
 private:
   const MachineInstr *MI;
 };
index 29478e9..cdcac1f 100644 (file)
@@ -264,14 +264,12 @@ public:
         CacheFI(MF.getFrameInfo(), TRI) {}
 
   bool process(MachineInstr &MI) {
-    unsigned VarIdx = StatepointOpers(&MI).getVarIdx();
-    uint64_t Flags =
-        MI.getOperand(VarIdx + StatepointOpers::FlagsOffset).getImm();
+    StatepointOpers SO(&MI);
+    uint64_t Flags = SO.getFlags();
     // Do nothing for LiveIn, it supports all registers.
     if (Flags & (uint64_t)StatepointFlags::DeoptLiveIn)
       return false;
-    CallingConv::ID CC =
-        MI.getOperand(VarIdx + StatepointOpers::CCOffset).getImm();
+    CallingConv::ID CC = SO.getCallingConv();
     const uint32_t *Mask = TRI.getCallPreservedMask(MF, CC);
     CacheFI.reset();
     StatepointState SS(MI, Mask, CacheFI);
index 44fb454..cc9dab6 100644 (file)
@@ -1551,26 +1551,27 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
     }
     break;
   }
-  case TargetOpcode::STATEPOINT:
-    if (!MI->getOperand(StatepointOpers::IDPos).isImm() ||
-        !MI->getOperand(StatepointOpers::NBytesPos).isImm() ||
-        !MI->getOperand(StatepointOpers::NCallArgsPos).isImm())
+  case TargetOpcode::STATEPOINT: {
+    StatepointOpers SO(MI);
+    if (!MI->getOperand(SO.getIDPos()).isImm() ||
+        !MI->getOperand(SO.getNBytesPos()).isImm() ||
+        !MI->getOperand(SO.getNCallArgsPos()).isImm()) {
       report("meta operands to STATEPOINT not constant!", MI);
-    break;
+      break;
+    }
 
     auto VerifyStackMapConstant = [&](unsigned Offset) {
-      if (!MI->getOperand(Offset).isImm() ||
-          MI->getOperand(Offset).getImm() != StackMaps::ConstantOp ||
-          !MI->getOperand(Offset + 1).isImm())
+      if (!MI->getOperand(Offset - 1).isImm() ||
+          MI->getOperand(Offset - 1).getImm() != StackMaps::ConstantOp ||
+          !MI->getOperand(Offset).isImm())
         report("stack map constant to STATEPOINT not well formed!", MI);
     };
-    const unsigned VarStart = StatepointOpers(MI).getVarIdx();
-    VerifyStackMapConstant(VarStart + StatepointOpers::CCOffset);
-    VerifyStackMapConstant(VarStart + StatepointOpers::FlagsOffset);
-    VerifyStackMapConstant(VarStart + StatepointOpers::NumDeoptOperandsOffset);
+    VerifyStackMapConstant(SO.getCCIdx());
+    VerifyStackMapConstant(SO.getFlagsIdx());
+    VerifyStackMapConstant(SO.getNumDeoptArgsIdx());
 
     // TODO: verify we have properly encoded deopt arguments
-    break;
+  } break;
   }
 }