<html>
<head>
<base href="https://bugs.llvm.org/">
</head>
<body><table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Bug ID</th>
<td><a class="bz_bug_link
bz_status_NEW "
title="NEW - [MachineBlockPlacement] Erroneous assert"
href="https://bugs.llvm.org/show_bug.cgi?id=42087">42087</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>[MachineBlockPlacement] Erroneous assert
</td>
</tr>
<tr>
<th>Product</th>
<td>libraries
</td>
</tr>
<tr>
<th>Version</th>
<td>trunk
</td>
</tr>
<tr>
<th>Hardware</th>
<td>PC
</td>
</tr>
<tr>
<th>OS</th>
<td>All
</td>
</tr>
<tr>
<th>Status</th>
<td>NEW
</td>
</tr>
<tr>
<th>Severity</th>
<td>enhancement
</td>
</tr>
<tr>
<th>Priority</th>
<td>P
</td>
</tr>
<tr>
<th>Component</th>
<td>Common Code Generator Code
</td>
</tr>
<tr>
<th>Assignee</th>
<td>unassignedbugs@nondot.org
</td>
</tr>
<tr>
<th>Reporter</th>
<td>james@jamesmolloy.co.uk
</td>
</tr>
<tr>
<th>CC</th>
<td>llvm-bugs@lists.llvm.org
</td>
</tr></table>
<p>
<div>
<pre>In asserts mode, the following code is enabled in MachineBlockPlacement (line
2476):
#ifndef NDEBUG
if (!BlocksWithUnanalyzableExits.count(PrevBB)) {
// Given the exact block placement we chose, we may actually not _need_
to
// be able to edit PrevBB's terminator sequence, but not being _able_ to
// do that at this point is a bug.
assert((!TII->analyzeBranch(*PrevBB, TBB, FBB, Cond) ||
!PrevBB->canFallThrough()) &&
"Unexpected block with un-analyzable fallthrough!");
Cond.clear();
TBB = FBB = nullptr;
}
#endif
This assertion is incorrect, or at least, can trigger on well-formed code.
BlocksWithUnanalyzableExits is populated with all blocks for which
analyzeBranch returns failure (true) or !BB->canFallThrough().
Consider an unanalyzable block that before layout is the last block in its
function. BB->canFallThrough() will *always* return false. Now consider the
above assert occurs *after* some layout has been changed. Now BB is no longer
the last BB in the block, canFallThrough() no longer returns false (because
it's unanalyzable and there's a succeessor block in layout order). We fail the
assert.
The comment below the assert appears to notice this can happen, but I don't
think it's got this particular case (the comment assumes the branch was
analyzable!):
// The "PrevBB" is not yet updated to reflect current code layout, so,
// o. it may fall-through to a block without explicit "goto" instruction
// before layout, and no longer fall-through it after layout; or
// o. just opposite.
//
// analyzeBranch() may return erroneous value for FBB when these two
// situations take place. For the first scenario FBB is mistakenly set
NULL;
// for the 2nd scenario, the FBB, which is expected to be NULL, is
// mistakenly pointing to "*BI".
// Thus, if the future change needs to use FBB before the layout is set, it
// has to correct FBB first by using the code similar to the following:
//
// if (!Cond.empty() && (!FBB || FBB == ChainBB)) {
// PrevBB->updateTerminator();
// Cond.clear();
// TBB = FBB = nullptr;
// if (TII->analyzeBranch(*PrevBB, TBB, FBB, Cond)) {
// // FIXME: This should never take place.
// TBB = FBB = nullptr;
// }
// }
if (!TII->analyzeBranch(*PrevBB, TBB, FBB, Cond))
PrevBB->updateTerminator();</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>