[TwoAddressInstruction] Don't run machine verification on unclean targets

[foad]

Summary

When -early-live-intervals is used, TwoAddressInstruction runs some
extra ad hoc machine verification, but it does this even for targets
that are marked as "not machine verifier clean". Fix this so that we
don't get spurious failures that are not related to live intervals.

[fhahn]

Sounds good to me. Is it possible to add a test case?

[foad]

Add test coverage.

[foad]

This is the only way I could think of to test it. Without the change to TwoAddressInstructionPass, enabling -early-live-intervals on Lanai (an unclean target) would throw up known errors like:

*** Bad machine code: MBB exits via conditional branch/fall-through but ends with a barrier instruction! ***
- function:    clz32
- basic block: %bb.0  (0x73a5a68) [0B;64B)

[fhahn]

LGTM, thanks!

[MatzeB]

llvm/lib/CodeGen/TwoAddressInstructionPass.cpp

1659–1665

Could we just remove this code? Doesn't seem like we miss much since this is the last step of the pass before we would have a verifier pass scheduled anyway...

[foad]

llvm/lib/CodeGen/TwoAddressInstructionPass.cpp

1659–1665

There is actually a subtle difference: if this pass is the last user of an analysis like LiveVariables, then calling MF->verify() here will verify that analysis, but scheduling a MachineVerifier pass afterwards will not, because the pass manager frees analyses immediately after their last user.

Because of that, this call to MF->verify() catches more problems than -verify-machineinstrs (or enabling LLVM_ENABLE_EXPENSIVE_CHECKS) does.

But I'd still be happy to remove it. I'll prepare a patch...

[foad]

Superseded by D111618.

llvm/lib/CodeGen/TwoAddressInstructionPass.cpp

1659–1665

D111618.

llvm/lib/CodeGen/TwoAddressInstructionPass.cpp

for (MachineBasicBlock::iterator mi = MBB->begin(), me = MBB->end();
 
       // Clear TiedOperands here instead of at the top of the loop
       // since most instructions do not have tied operands.
       TiedOperands.clear();
       mi = nmi;
     }
   }
 
-  if (LIS)
+  // TODO: Remove this ad hoc verification in favor of the usual
+  // -verify-machineinstrs mechanism.
+  if (LIS) {
+    const LLVMTargetMachine &TM = MF->getTarget();
+    if (TM.isMachineVerifierClean())
       MF->verify(this, "After two-address instruction pass");
+  }
 
   return MadeChange;
 }
 
 /// Eliminate a REG_SEQUENCE instruction as part of the de-ssa process.
 ///
 /// The instruction is turned into a sequence of sub-register copies:
 ///

llvm/test/CodeGen/Lanai/i32.ll

 ; RUN: llc < %s -asm-verbose=false | FileCheck %s
+; RUN: llc < %s -asm-verbose=false -early-live-intervals | FileCheck %s
 
 ; Test that basic 32-bit integer operations assemble as expected.
 
 target datalayout = "E-m:e-p:32:32-i64:64-a:0:32-n32-S64"
 target triple = "lanai"
 
 ; Function Attrs: nounwind readnone
 declare i32 @llvm.ctpop.i32(i32) #1