<html><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8" /></head><body style='font-size: 10pt; font-family: Verdana,Geneva,sans-serif'>
<p><br /></p>
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">I took a look at 3.9 and 4.0 code. Now, here's what I understood:</div>
<ul>
<li class="gmail_quote">The IfConversion in 3.9 is interesting as it checks <em>IsBrAnalyzable()</em> for TrueBB and FalseBB before calling <em>removeBranch()</em>.</li>
<li class="gmail_quote">The comments and code of <em>analyzeBranch()</em> and <em>removeBranch()</em> are clear to me now. My previous fix inside removeBranch() was definitely not correct.</li>
<li class="gmail_quote">Based on the comments (for example : "a join block may not be present") and the code, a diamond does not have to be a complete diamond. The TailBB is not mandatory.</li>
</ul>
<p>Conclusion: My case should be supported then. </p>
<p><span class="s1"><em>IfConvertDiamondCommon() </em>support different scenarios for a given EntryBB. A quick summary:</span></p>
<ol>
<li><span class="s1">FalseBB and TrueBB contains both an analyzable branch. <em>SkipUnconditionalBranches=true</em> meaning the branches are not checked if they are identical. <em>ValidDiamond()</em> checks that they end up on the same TailBB. If TailBB is either nullptr or not the same for both TrueBB and FalseBB, the diamond is not valid. <em>IfConvertDiamondCommon()</em> is called with removeBranch=true. Branches are successfully removed in both blocks, thus common instructions are correctly removed based on Dups2 value. At the end, TailBB is merged or a conditional branch is inserted. <span>IfConversion successful.</span></span></li>
<li><span class="s1">FalseBB not analyzable and TrueBB <span>analyzable or inversely (inducing a different branch instruction). <em>ValidDiamond()</em> is going to fail since FalseBB/TrueBB.TrueBB will be equal to nullptr for the not analyzable one hence <em>TT =! FT </em>will trigger. </span></span></li>
<li><span class="s1"><span>FalseBB and TrueBB both not analyzable (indirect branch for example on ARM). So <em>SkipUnconditionalBranches=false.</em></span></span>
<ol>
<li>If the branches are not identical, <em>CountDuplicatedInstruction</em> will set Dups2 to 0<em>. </em>Then<em> IfConvertDiamondCommon()</em> is called with removeBranch=false. The <em>removeBranch()</em> on TrueBB/BBI1 will silently fail. TrueBB will be predicated completely (branch instruction included). FalseBB on the other hand will be predicated excluding the branch instruction (due to the loop on DI2 when removeBranch=false). IfConversion successful.</li>
<li>If the branches are identical. <em>CountDuplicatedInstruction</em><span> will set Dups2 accordingly.</span>
<ul>
<li><span>If there's no common instruction, same behaviour as in 3.1</span></li>
<li><span>If there's some common instructions, this is my case !</span></li>
</ul>
</li>
</ol>
</li>
</ol>
<p>In my case: The EntryBB is analyzable but TrueBB/FalseBB are not (<em>AnalyzeBranches()</em>). TrueBB/FalseBB instructions can be predicated (<em>ScanInstruction()</em>. <em>feasibleDiamond()</em>). TrueBB/FalseBB cannot <em>SkipUnconditionalBranches</em> (<em>ValidDiamond()</em>). <em>CountDuplicatedInstruction()</em> compute Dups1=0 as there's no common instructions found at the beginning and Dups2=1 as it found one common instruction at the end. During their computation, neither debug instructions (<span class="s1"><em>skipDebugInstructionsForward()</em>) nor branch instructions (<em>TIB->isBranch()</em>) are counted.</span> We note that because TrueBB and FalseBB are not analyzable, <em>CountDuplicatedInstruction()</em> also ensure that the branch instruction is exactly the same in both blocks. If not, it would not fail but simply set Dups2 to 0.</p>
<p>Now <span class="s1">when the blocks are prepared to be merged in <em>IfConvertDiamondCommon()</em>, <em>removeBranch()</em> fails silently on TrueBB (BBI1) as indirect branches are not supported. Hence the erasing of the common instructions based on Dups2=1 value will produce an invalid block because it will remove the branch instead of the actual common instruction.</span></p>
<p><span class="s1">In the patch below, I manually delete the branch if and only if they are identical in both block TrueBB/FalseBB. I also removed the call to <span><em>verifySameBranchInstructions()</em> as the blocks do not have to have identical branch as explained in point 3.1 above. Works fine</span></span></p>
<blockquote type="cite" style="padding: 0 0.4em; border-left: #1010ff 2px solid; margin: 0">
<p><span class="s1">diff --git a/lib/CodeGen/IfConversion.cpp b/lib/CodeGen/IfConversion.cpp<br />index ff840536617..8b8f4e67242 100644<br />--- a/lib/CodeGen/IfConversion.cpp<br />+++ b/lib/CodeGen/IfConversion.cpp<br />@@ -1781,14 +1747,25 @@ bool IfConverter::IfConvertDiamondCommon(<br /> BBI.BB->splice(BBI.BB->end(), &MBB1, MBB1.begin(), DI1);<br /> MBB2.erase(MBB2.begin(), DI2);<br /> <br />- // The branches have been checked to match, so it is safe to remove the branch<br />- // in BB1 and rely on the copy in BB2<br />-#ifndef NDEBUG<br />- // Unanalyzable branches must match exactly. Check that now.<br />- if (!BBI1->IsBrAnalyzable)<br />- verifySameBranchInstructions(&MBB1, &MBB2);<br />-#endif<br />- BBI1->NonPredSize -= TII->removeBranch(*BBI1->BB);<br />+ // When analyzable, the branches have been checked to match, so it is safe to<br />+ // remove the branch in BB1 and rely on the copy in BB2<br />+ // For non-analyzable, remove branch only if they are identical<br />+ if(BBI1->IsBrAnalyzable)<br />+ BBI1->NonPredSize -= TII->removeBranch(*BBI1->BB);<br />+ else {<br />+ DI1 = BBI1->BB->end();<br />+ DI2 = BBI2->BB->end();<br />+ do {<br />+ --DI1;<br />+ }while(!DI1->isBranch() && DI1 != BBI1->BB->begin());<br />+ do {<br />+ --DI2;<br />+ }while(!DI2->isBranch() && DI2 != BBI2->BB->begin());<br />+ <br />+ if(DI1->isIdenticalTo(*DI2))<br />+ DI1->eraseFromParent();<br />+ }<br />+<br /> // Remove duplicated instructions.<br /> DI1 = MBB1.end();<br /> for (unsigned i = 0; i != NumDups2; ) {<br /> </span></p>
</blockquote>
<p><br /></p>
</div>
</div>
<p><br /></p>
</body></html>