[llvm] edbb27c - [Statepoint] Add getters to StatepointOpers.

[Denis Antrushin via llvm-commits]

Author: Denis Antrushin
Date: 2020-04-15T14:31:42+03:00
New Revision: edbb27ccb63402b591a459f4087434ea778c23a7

URL: https://github.com/llvm/llvm-project/commit/edbb27ccb63402b591a459f4087434ea778c23a7
DIFF: https://github.com/llvm/llvm-project/commit/edbb27ccb63402b591a459f4087434ea778c23a7.diff

LOG: [Statepoint] Add getters to StatepointOpers.

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/StackMaps.h b/llvm/include/llvm/CodeGen/StackMaps.h
index 63547e5b7c3e..e33ee226e41a 100644
--- a/llvm/include/llvm/CodeGen/StackMaps.h
+++ b/llvm/include/llvm/CodeGen/StackMaps.h
@@ -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 @@ class StatepointOpers {
     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;
 };

diff  --git a/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp b/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
index 29478e9eeec9..cdcac1f50db6 100644
--- a/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
+++ b/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
@@ -264,14 +264,12 @@ class StatepointProcessor {
         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);

diff  --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 44fb45484a54..cc9dab6db913 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -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;
   }
 }