[mips] Change the machine verifier to allow nested call sequences.

[slthakur]

Summary

The call to the function f2 in the test largeimmprinting.ll has a large size argument passed by value. This causes the lowering implementation to create a call to memcpy for coping the large sized argument in the argument build area.

This results in a nested call sequence which was not allowed by the machine verifier before. This patch adds a stack to save the sizes of the stack adjustments and checks if there is any mismatch.

[sdardis]

Hi Sagar,

I don't think this is the correct fix. The issue arising is that the call to memcpy is being generated inside the callseq_start, callseq_end nodes in the DAG. Instead, when we encounter a byval argument, we need to call memcpy outside the sequence. If you check this test with xcore/x86_64/i386/aarch64 you'll see that those targets also call out to memcpy as well. We should look at changing how the arguments are setup for a call in MipsISelLowering.cpp.

The ISD also notes that those callseq nodes are not to be nested.

Thanks,
Simon

lib/CodeGen/MachineVerifier.cpp

struct StackStateOfBB {
     StackStateOfBB(int EntryVal, int ExitVal, bool EntrySetup, bool ExitSetup) :
       EntryValue(EntryVal), ExitValue(ExitVal), EntryIsSetup(EntrySetup),
       ExitIsSetup(ExitSetup) { }
     // Can be negative, which means we are setting up a frame.
     int EntryValue;
     int ExitValue;
     bool EntryIsSetup;
     bool ExitIsSetup;
+    std::stack<int> FrameSetupSize;
   };
 }
 
 /// Make sure on every path through the CFG, a FrameSetup <n> is always followed
 /// by a FrameDestroy <n>, stack adjustments are identical on all
 /// CFG edges to a merge point, and frame is destroyed at end of a return block.
 void MachineVerifier::verifyStackFrame() {
   unsigned FrameSetupOpcode   = TII->getCallFrameSetupOpcode();
for (df_ext_iterator<const MachineFunction*,
     if (DFI.getPathLength() >= 2) {
       const MachineBasicBlock *StackPred = DFI.getPath(DFI.getPathLength() - 2);
       assert(Reachable.count(StackPred) &&
              "DFS stack predecessor is already visited.\n");
       BBState.EntryValue = SPState[StackPred->getNumber()].ExitValue;
       BBState.EntryIsSetup = SPState[StackPred->getNumber()].ExitIsSetup;
       BBState.ExitValue = BBState.EntryValue;
       BBState.ExitIsSetup = BBState.EntryIsSetup;
+      BBState.FrameSetupSize = SPState[StackPred->getNumber()].FrameSetupSize;
     }
 
     // Update stack state by checking contents of MBB.
     for (const auto &I : *MBB) {
       if (I.getOpcode() == FrameSetupOpcode) {
         // The first operand of a FrameOpcode should be i32.
         int Size = I.getOperand(0).getImm();
         assert(Size >= 0 &&
           "Value should be non-negative in FrameSetup and FrameDestroy.\n");
 
-        if (BBState.ExitIsSetup)
-          report("FrameSetup is after another FrameSetup", &I);
         BBState.ExitValue -= Size;
         BBState.ExitIsSetup = true;
+        BBState.FrameSetupSize.push(Size);
       }
 
       if (I.getOpcode() == FrameDestroyOpcode) {
         // The first operand of a FrameOpcode should be i32.
         int Size = I.getOperand(0).getImm();
         assert(Size >= 0 &&
           "Value should be non-negative in FrameSetup and FrameDestroy.\n");
 
-        if (!BBState.ExitIsSetup)
-          report("FrameDestroy is not after a FrameSetup", &I);
-        int AbsSPAdj = BBState.ExitValue < 0 ? -BBState.ExitValue :
-                                               BBState.ExitValue;
-        if (BBState.ExitIsSetup && AbsSPAdj != Size) {
+        if (!BBState.FrameSetupSize.empty())
+        {
+          if (BBState.FrameSetupSize.top() != Size) {
             report("FrameDestroy <n> is after FrameSetup <m>", &I);
             errs() << "FrameDestroy <" << Size << "> is after FrameSetup <"
-              << AbsSPAdj << ">.\n";
+                << BBState.FrameSetupSize.top() << ">.\n";
           }
+          BBState.FrameSetupSize.pop();
+        }
+        else
+          report("FrameDestroy is not after a FrameSetup", &I);
         BBState.ExitValue += Size;
         BBState.ExitIsSetup = false;
       }
     }
     SPState[MBB->getNumber()] = BBState;
 
     // Make sure the exit state of any predecessor is consistent with the entry
     // state.

test/CodeGen/Mips/largeimmprinting.ll

-; RUN: llc -march=mipsel -relocation-model=pic < %s | FileCheck %s -check-prefix=32
-; RUN: llc -march=mips64el -mcpu=mips4 -target-abi=n64 -relocation-model=pic < %s | \
+; RUN: llc -march=mipsel -relocation-model=pic -verify-machineinstrs < %s | FileCheck %s -check-prefix=32
+; RUN: llc -march=mips64el -mcpu=mips4 -target-abi=n64 -relocation-model=pic -verify-machineinstrs < %s | \
 ; RUN:     FileCheck %s -check-prefix=64
-; RUN: llc -march=mips64el -mcpu=mips64 -target-abi=n64 -relocation-model=pic < %s | \
+; RUN: llc -march=mips64el -mcpu=mips64 -target-abi=n64 -relocation-model=pic -verify-machineinstrs < %s | \
 ; RUN:     FileCheck %s -check-prefix=64
 
 %struct.S1 = type { [65536 x i8] }
 
 @s1 = external global %struct.S1
 
 define void @f() nounwind {
 entry: