<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 - [IfConversion] Diamond assumes unanalyzable dests are equivalent"
href="https://bugs.llvm.org/show_bug.cgi?id=43800">43800</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>[IfConversion] Diamond assumes unanalyzable dests are equivalent
</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>Windows NT
</td>
</tr>
<tr>
<th>Status</th>
<td>NEW
</td>
</tr>
<tr>
<th>Severity</th>
<td>normal
</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>alex.davies@iinet.net.au
</td>
</tr>
<tr>
<th>CC</th>
<td>llvm-bugs@lists.llvm.org
</td>
</tr></table>
<p>
<div>
<pre>ValidDiamond considers if-converting sub-CFGs like:
// Diamond:
// EBB
// / \_
// | |
// TBB FBB
// \ /
// TailBB
But does not check the actual successors of TBB/FBB when merging, but rather
previously analyzed results.
The problem with that is that if TBB and FBB _both_ fail to be analyzed, it's
comparing nullptr with nullptr, considers TailBB(nullptr) equal, and evaluates
it for a (code-breaking) merge.
***
Relevant code segments:
TrueBB/FalseBB are nullptr on failure to analyze:
if (!BBI.IsBrAnalyzable) {
BBI.TrueBB = nullptr;
BBI.FalseBB = nullptr;
BBI.BrCond.clear();
}
ValidDiamond:
MachineBasicBlock *TT = TrueBBI.TrueBB;
MachineBasicBlock *FT = FalseBBI.TrueBB;
if (!TT && blockAlwaysFallThrough(TrueBBI))
TT = getNextBlock(*TrueBBI.BB);
if (!FT && blockAlwaysFallThrough(FalseBBI))
FT = getNextBlock(*FalseBBI.BB);
if (TT != FT)
return false;
(blockAlwaysFallThrough returns false if not analyzable, leaving TT and FT as
nullptr)
***
What is odd to me, is that ValidDiamond seems _designed_ to try and operate
with non-analyzable branches, without even checking CFG successors.
I can understand this in the case of 0 successors (ret/tailcall) or _maybe_
where analyzeBranch() fails but each block has the same, single, successor (I
wouldn't go beyond that myself)... but to not have any checks at all is just
asking for trouble. Apologies for lack of demonstration code, it's hard to
reproduce without familiarity with an in-tree architecture and their
analyzeBranch results, but I am 99% sure there is a real problem here (that/or
I've misimplemented analyzeBranch on the arch I'm on...).</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>