[llvm] r343731 - [machineverifier] Detect PHI's that are preceeded by non-PHI's

[Daniel Sanders via llvm-commits]

Author: dsanders
Date: Wed Oct  3 15:05:31 2018
New Revision: 343731

URL: http://llvm.org/viewvc/llvm-project?rev=343731&view=rev
Log:
[machineverifier] Detect PHI's that are preceeded by non-PHI's

If present, PHI nodes must appear before non-PHI nodes in a basic block. The
register allocator relies on this and will fail to eliminate PHI's that do not
meet this requirement.


Added:
    llvm/trunk/test/Verifier/test_phis_precede_nonphis.mir
Modified:
    llvm/trunk/lib/CodeGen/MachineVerifier.cpp

Modified: llvm/trunk/lib/CodeGen/MachineVerifier.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineVerifier.cpp?rev=343731&r1=343730&r2=343731&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineVerifier.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineVerifier.cpp Wed Oct  3 15:05:31 2018
@@ -109,6 +109,7 @@ namespace {
     using RegMap = DenseMap<unsigned, const MachineInstr *>;
     using BlockSet = SmallPtrSet<const MachineBasicBlock *, 8>;
 
+    const MachineInstr *FirstNonPHI;
     const MachineInstr *FirstTerminator;
     BlockSet FunctionBlocks;
 
@@ -608,6 +609,7 @@ static bool matchPair(MachineBasicBlock:
 void
 MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) {
   FirstTerminator = nullptr;
+  FirstNonPHI = nullptr;
 
   if (!MF->getProperties().hasProperty(
       MachineFunctionProperties::Property::NoPHIs) && MRI->tracksLiveness()) {
@@ -889,9 +891,15 @@ void MachineVerifier::visitMachineInstrB
            << MI->getNumOperands() << " given.\n";
   }
 
-  if (MI->isPHI() && MF->getProperties().hasProperty(
-                         MachineFunctionProperties::Property::NoPHIs))
-    report("Found PHI instruction with NoPHIs property set", MI);
+  if (MI->isPHI()) {
+    if (MF->getProperties().hasProperty(
+            MachineFunctionProperties::Property::NoPHIs))
+      report("Found PHI instruction with NoPHIs property set", MI);
+
+    if (FirstNonPHI)
+      report("Found PHI instruction after non-PHI", MI);
+  } else if (FirstNonPHI == nullptr)
+    FirstNonPHI = MI;
 
   // Check the tied operands.
   if (MI->isInlineAsm())

Added: llvm/trunk/test/Verifier/test_phis_precede_nonphis.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Verifier/test_phis_precede_nonphis.mir?rev=343731&view=auto
==============================================================================
--- llvm/trunk/test/Verifier/test_phis_precede_nonphis.mir (added)
+++ llvm/trunk/test/Verifier/test_phis_precede_nonphis.mir Wed Oct  3 15:05:31 2018
@@ -0,0 +1,84 @@
+# RUN: not llc -run-pass=machineverifier %s -o - 2>&1 | FileCheck %s
+# REQUIRES: aarch64-registered-target
+
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64--"
+  define void @valid(i8* %addr) {
+  entry:
+    br i1 0, label %if, label %else
+  if:
+    br label %exit
+  else:
+    br label %exit
+  exit:
+    ret void
+  }
+
+  define void @broken(i8* %addr) {
+  entry:
+    br i1 0, label %if, label %exit
+  if:
+    br label %exit
+  exit:
+    ret void
+  }
+...
+
+---
+name:            valid
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $w1
+    successors: %bb.1(0x40000000), %bb.2(0x40000000); %bb.1(50.00%), %bb.2(50.00%)
+    %1:_(s32) = COPY $w1
+    %2:_(s32) = G_CONSTANT i32 1
+    %3:_(s1) = G_ICMP intpred(ne), %1:_(s32), %2:_
+    G_BRCOND %3:_(s1), %bb.1
+    G_BR %bb.2.else
+  bb.1.if:
+    successors: %bb.3(0x80000000)
+    %4:_(s32) = G_CONSTANT i32 1
+    %5:_(s8) = G_TRUNC %4
+    G_BR %bb.3.exit
+  bb.2.else:
+    successors: %bb.3(0x80000000)
+    %6:_(s32) = G_CONSTANT i8 1
+    %7:_(s8) = G_TRUNC %6
+    G_BR %bb.3.exit
+  bb.3.exit:
+    %8:_(s8) = G_PHI %5:_(s8), %bb.1, %7:_(s8), %bb.2
+    %9:_(s32) = G_ZEXT %8
+    $w1 = COPY %9
+...
+
+---
+name:            broken
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $w1
+    successors: %bb.1(0x40000000), %bb.2(0x40000000); %bb.1(50.00%), %bb.2(50.00%)
+    %1:_(s32) = COPY $w1
+    %2:_(s32) = G_CONSTANT i32 1
+    %3:_(s1) = G_ICMP intpred(ne), %1:_(s32), %2:_
+    %4:_(s32) = G_CONSTANT i32 1
+    %6:_(s8) = G_CONSTANT i8 2
+    G_BRCOND %3:_(s1), %bb.1
+    G_BR %bb.2.exit
+  bb.1.if:
+    successors: %bb.2(0x80000000)
+    G_BR %bb.2.exit
+  bb.2.exit:
+    %5:_(s8) = G_TRUNC %4
+    %8:_(s8) = G_PHI %5:_(s8), %bb.0, %6:_(s8), %bb.1
+    %9:_(s32) = G_ZEXT %8
+    $w1 = COPY %9
+...
+# CHECK-NOT: Bad machine code
+# CHECK-LABEL: Bad machine code: Found PHI instruction after non-PHI
+# CHECK-NEXT:  - function:    broken
+# CHECK-NEXT:  - basic block: %bb.2 exit
+# CHECK-NEXT:  - instruction: %6:_(s8) = G_PHI %5:_(s8), %bb.0, %4:_(s8), %bb.1
+# CHECK-NOT: Bad machine code