Add placeholder for more extensive verification of psuedo ops

[reames]

Summary

This initial patch doesn't actually do much useful. It's just to show where the new code goes. Once this is in, I'll extend the verification logic to check more useful properties.

For those curious, the more complicated version of this patch already found one very suspicious thing.

p.s. At the time of posting, this hasn't actually finished building. For some reason my incremental build decided to do a full rebuild which takes a while. That's not the main point of the review anyways; I'm mostly curious about placement/purpose comments.

[reames]

I tempted fate, so of course the rushed patch didn't build. Oy.

[MatzeB]

Not much to say at this point except: More machine verifiers are a good thing here esp. since those patchpoint related instructions are variadic/generic enough that our usual MCInstrDesc mechanism don't do much anymore.

lib/CodeGen/MachineVerifier.cpp

926–928 ↗

(On Diff #101164)

You can probably move the TargetOpcode::G_LOAD/TargetOpcode::G_STORE cases from above into the switch as well?

llvm/trunk/include/llvm/CodeGen/StackMaps.h

 ///
 /// Statepoint operands take the form:
 ///   <id>, <num patch bytes >, <num call arguments>, <call target>,
 ///   [call arguments], <StackMaps::ConstantOp>, <calling convention>,
 ///   <StackMaps::ConstantOp>, <statepoint flags>,
 ///   <StackMaps::ConstantOp>, <num other args>, [other args],
 ///   [gc values]
 class StatepointOpers {
-private:
+public:
   // These values are aboolute 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
   // arguments (i.e. the end of the call arguments).
   enum { CCOffset = 1, FlagsOffset = 3, NumVMSArgsOffset = 5 };
 
-public:
   explicit StatepointOpers(const MachineInstr *MI) : MI(MI) {}
 
   /// 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;
   }
 

llvm/trunk/lib/CodeGen/MachineVerifier.cpp

 #include "llvm/Analysis/EHPersonalities.h"
 #include "llvm/CodeGen/LiveIntervalAnalysis.h"
 #include "llvm/CodeGen/LiveStackAnalysis.h"
 #include "llvm/CodeGen/LiveVariables.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineMemOperand.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
+#include "llvm/CodeGen/StackMaps.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/raw_ostream.h"
void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
   // Generic opcodes must not have physical register operands.
   if (isPreISelGenericOpcode(MCID.getOpcode())) {
     for (auto &Op : MI->operands()) {
       if (Op.isReg() && TargetRegisterInfo::isPhysicalRegister(Op.getReg()))
         report("Generic instruction cannot have physical register", MI);
     }
   }
 
+  StringRef ErrorInfo;
+  if (!TII->verifyInstruction(*MI, ErrorInfo))
+    report(ErrorInfo.data(), MI);
+
+  // Verify properties of various specific instruction types
+  switch(MI->getOpcode()) {
+  default:
+    break;
+  case TargetOpcode::G_LOAD:
+  case TargetOpcode::G_STORE:
     // Generic loads and stores must have a single MachineMemOperand
     // describing that access.
-  if ((MI->getOpcode() == TargetOpcode::G_LOAD ||
-       MI->getOpcode() == TargetOpcode::G_STORE) &&
-      !MI->hasOneMemOperand())
+    if (!MI->hasOneMemOperand())
       report("Generic instruction accessing memory must have one mem operand",
              MI);
-
-  StringRef ErrorInfo;
-  if (!TII->verifyInstruction(*MI, ErrorInfo))
-    report(ErrorInfo.data(), MI);
+    break;
+  case TargetOpcode::STATEPOINT:
+    if (!MI->getOperand(StatepointOpers::IDPos).isImm() ||
+        !MI->getOperand(StatepointOpers::NBytesPos).isImm() ||
+        !MI->getOperand(StatepointOpers::NCallArgsPos).isImm())
+      report("meta operands to STATEPOINT not constant!", MI);
+    break;
+  };
 }
 
 void
 MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) {
   const MachineInstr *MI = MO->getParent();
   const MCInstrDesc &MCID = MI->getDesc();
   unsigned NumDefs = MCID.getNumDefs();
   if (MCID.getOpcode() == TargetOpcode::PATCHPOINT)

llvm/trunk/test/CodeGen/X86/statepoint-allocas.ll

-; RUN: llc < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs < %s | FileCheck %s
 ; Check that we can lower a use of an alloca both as a deopt value (where the
 ; exact meaning is up to the consumer of the stackmap) and as an explicit spill
 ; slot used for GC.  
 
 target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-linux-gnu"
 
 declare zeroext i1 @return_i1()

llvm/trunk/test/CodeGen/X86/statepoint-call-lowering.ll

-; RUN: llc < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs < %s | FileCheck %s
 ; This file contains a collection of basic tests to ensure we didn't
 ; screw up normal call lowering when there are no deopt or gc arguments.
 
 target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-linux-gnu"
 
 %struct = type { i64, i64 }
 

llvm/trunk/test/CodeGen/X86/statepoint-far-call.ll

-; RUN: llc < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs < %s | FileCheck %s
 ; Test to check that Statepoints with X64 far-immediate targets
 ; are lowered correctly to an indirect call via a scratch register.
 
 target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-win64"
 
 define void @test_far_call() gc "statepoint-example" {
 ; CHECK-LABEL: test_far_call

llvm/trunk/test/CodeGen/X86/statepoint-forward.ll

 ; RUN: opt -O3 -S < %s | FileCheck --check-prefix=CHECK-OPT %s
-; RUN: llc < %s | FileCheck --check-prefix=CHECK-LLC %s
+; RUN: llc -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK-LLC %s
 ; These tests are targetted at making sure we don't retain information
 ; about memory which contains potential gc references across a statepoint.
 ; They're carefully written to only outlaw forwarding of references. 
 ; Depending on the collector, forwarding non-reference fields or
 ; constant null references may be perfectly legal. (If unimplemented.)
 ; The general structure of these tests is:
 ; - learn a fact about memory (via an assume)
 ; - cross a statepoint

llvm/trunk/test/CodeGen/X86/statepoint-gctransition-call-lowering.ll

-; RUN: llc < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs < %s | FileCheck %s
 ; This file contains a collection of basic tests to ensure we didn't
 ; screw up normal call lowering when a statepoint is a GC transition.
 
 target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-linux-gnu"
 
 declare zeroext i1 @return_i1()
 declare zeroext i32 @return_i32()

llvm/trunk/test/CodeGen/X86/statepoint-invoke.ll

-; RUN: llc < %s 2>&1 | FileCheck %s
+; RUN: llc -verify-machineinstrs < %s 2>&1 | FileCheck %s
 
 target triple = "x86_64-pc-linux-gnu"
 
 declare void @"some_call"(i64 addrspace(1)*)
 declare i64 addrspace(1)* @"some_other_call"(i64 addrspace(1)*)
 
 declare i32 @"personality_function"()
 

llvm/trunk/test/CodeGen/X86/statepoint-live-in.ll

-; RUN: llc -O3 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -O3 < %s | FileCheck %s
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
 declare void @bar() #0
 declare void @baz()
 
 define void @test1(i32 %a) gc "statepoint-example" {
 entry:

llvm/trunk/test/CodeGen/X86/statepoint-stack-usage.ll

-; RUN: llc -stack-symbol-ordering=0 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -stack-symbol-ordering=0 < %s | FileCheck %s
 
 target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-linux-gnu"
 
 ; This test is checking to make sure that we reuse the same stack slots
 ; for GC values spilled over two different call sites.  Since the order
 ; of GC arguments differ, niave lowering code would insert loads and 
 ; stores to rearrange items on the stack.  We need to make sure (for

llvm/trunk/test/CodeGen/X86/statepoint-stackmap-format.ll

-; RUN: llc < %s -stack-symbol-ordering=0 -mtriple="x86_64-pc-linux-gnu" | FileCheck %s
-; RUN: llc < %s -stack-symbol-ordering=0 -mtriple="x86_64-pc-unknown-elf" | FileCheck %s
+; RUN: llc < %s -verify-machineinstrs -stack-symbol-ordering=0 -mtriple="x86_64-pc-linux-gnu" | FileCheck %s
+; RUN: llc < %s -verify-machineinstrs -stack-symbol-ordering=0 -mtriple="x86_64-pc-unknown-elf" | FileCheck %s
 
 ; This test is a sanity check to ensure statepoints are generating StackMap
 ; sections correctly.  This is not intended to be a rigorous test of the 
 ; StackMap format (see the stackmap tests for that).
 
 target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
 
 declare zeroext i1 @return_i1()

llvm/trunk/test/CodeGen/X86/statepoint-uniqueing.ll

-; RUN: llc < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs < %s | FileCheck %s
 
 ; Checks for a crash we had when two gc.relocate calls would
 ; relocating identical values
 
 target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-linux-gnu"
 
 declare void @f()

llvm/trunk/test/CodeGen/X86/statepoint-vector-bad-spill.ll

-; RUN: llc -O3 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -O3 < %s | FileCheck %s
 
 ; This is checking for a crash.
 
 target triple = "x86_64-pc-linux-gnu"
 
 define <2 x i8 addrspace(1)*> @test0(i8 addrspace(1)* %el, <2 x i8 addrspace(1)*>* %vec_ptr) gc "statepoint-example" {
 ; CHECK-LABEL: test0:
 

llvm/trunk/test/CodeGen/X86/statepoint-vector.ll

-; RUN: llc -stack-symbol-ordering=0 -mcpu=nehalem -debug-only=stackmaps < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -stack-symbol-ordering=0 -mcpu=nehalem -debug-only=stackmaps < %s | FileCheck %s
 ; REQUIRES: asserts
 
 target triple = "x86_64-pc-linux-gnu"
 
 ; Can we lower a single vector?
 define <2 x i8 addrspace(1)*> @test(<2 x i8 addrspace(1)*> %obj) gc "statepoint-example" {
 entry:
 ; CHECK-LABEL: @test