From 7fdaa5e64114bd5b614cff87bc350e4358b95200 Mon Sep 17 00:00:00 2001 From: Quentin Colombet Date: Tue, 19 Sep 2017 21:55:51 +0000 Subject: [PATCH] [MIRPrinter] Print empty successor lists when they cannot be guessed Unreachable blocks in the machine instr representation are these weird empty blocks with no successors. The MIR printer used to not print empty lists of successors. However, the MIR parser now treats non-printed list of successors as "please guess it for me". As a result, the parser tries to guess the list of successors and given the block is empty, just assumes it falls through the next block (if any). For instance, the following test case used to fail the verifier. The MIR printer would print entry / \ true (def) false (no list of successors) | split.true (use) The MIR parser would understand this: entry / \ true (def) false | / <-- invalid edge split.true (use) Because of the invalid edge, we get the "def does not dominate all uses" error. The fix consists in printing empty successor lists, so that the parser knows what to do for unreachable blocks. rdar://problem/34022159 llvm-svn: 313685 --- llvm/lib/CodeGen/MIRPrinter.cpp | 3 +- .../test/CodeGen/MIR/X86/unreachable_block.ll | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 llvm/test/CodeGen/MIR/X86/unreachable_block.ll diff --git a/llvm/lib/CodeGen/MIRPrinter.cpp b/llvm/lib/CodeGen/MIRPrinter.cpp index db9ccef8a365..6347e6da9422 100644 --- a/llvm/lib/CodeGen/MIRPrinter.cpp +++ b/llvm/lib/CodeGen/MIRPrinter.cpp @@ -598,8 +598,7 @@ void MIPrinter::print(const MachineBasicBlock &MBB) { bool HasLineAttributes = false; // Print the successors bool canPredictProbs = canPredictBranchProbabilities(MBB); - if (!MBB.succ_empty() && (!SimplifyMIR || !canPredictProbs || - !canPredictSuccessors(MBB))) { + if (!SimplifyMIR || !canPredictProbs || !canPredictSuccessors(MBB)) { OS.indent(2) << "successors: "; for (auto I = MBB.succ_begin(), E = MBB.succ_end(); I != E; ++I) { if (I != MBB.succ_begin()) diff --git a/llvm/test/CodeGen/MIR/X86/unreachable_block.ll b/llvm/test/CodeGen/MIR/X86/unreachable_block.ll new file mode 100644 index 000000000000..4cea0392255c --- /dev/null +++ b/llvm/test/CodeGen/MIR/X86/unreachable_block.ll @@ -0,0 +1,48 @@ +; RUN: llc -mtriple x86_64-- -stop-before peephole-opt -o %t.mir %s +; RUN: llc -mtriple x86_64-- -run-pass none %t.mir -verify-machineinstrs -o - | FileCheck %s + +; Unreachable blocks in the machine instr representation are these +; weird empty blocks with no successors. +; The MIR printer used to not print empty lists of successors. However, +; the MIR parser now treats non-printed list of successors as "please +; guess it for me". As a result, the parser tries to guess the list of +; successors and given the block is empty, just assumes it falls through +; the next block. +; +; The following test case used to fail the verifier because the false +; path ended up falling through split.true and now, the definition of +; %v does not dominate all its uses. +; Indeed, we go from the following CFG: +; entry +; / \ +; true (def) false +; | +; split.true (use) +; +; To this one: +; entry +; / \ +; true (def) false +; | / <-- invalid edge +; split.true (use) +; +; Because of the invalid edge, we get the "def does not +; dominate all uses" error. +; +; CHECK-LABEL: name: foo +; CHECK-LABEL: bb.{{[0-9]+}}.false: +; CHECK-NEXT: successors: +; CHECK-NOT: %bb.{{[0-9]+}}.split.true +; CHECK-LABEL: bb.{{[0-9]+}}.split.true: +define void @foo(i32* %bar) { + br i1 undef, label %true, label %false +true: + %v = load i32, i32* %bar + br label %split.true +false: + unreachable +split.true: + %vInc = add i32 %v, 1 + store i32 %vInc, i32* %bar + ret void +}