No worries at all - thanks for looking into it!<br><br><div class="gmail_quote"><div dir="ltr">On Tue., 28 Aug. 2018, 10:58 am Vedant Kumar, <<a href="mailto:vsk@apple.com">vsk@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space">Thanks for reverting Dave. Sorry for the breakage. This should be enough information for me to revise the patch & add a regression test.</div><div style="word-wrap:break-word;line-break:after-white-space"><div><br></div><div>vedant<br><div><br><blockquote type="cite"><div>On Aug 27, 2018, at 5:56 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br class="m_-5563556590994004418Apple-interchange-newline"><div><div dir="ltr">Looks like this is causing a crash - the pass later assumes that all the select instructions discovered are contiguous.<br><br>This resulted in a crash later in CodeGenPrep because the last instruction in the BB wasn't actually a terminator. A basic block ends like this (where the select had been replaced by the branch, but then the llvm.dbg.value was left after that select/br):<br><br><div>    br i1 %495, label ..., label ..., !dbg !251, !prof !252</div><div>    call void @llvm.dbg.value(metadata float ..., metadata !152, metadata !DIExpression()), !dbg !241<br><br>(specifically, the backtrace is:<br><br><div>llvm::dyn_cast<llvm::ReturnInst, llvm::TerminatorInst> (Val=0x0) at include/llvm/Support/Casting.h:334</div><div>CodeGenPrepare::dupRetToEnableTailCallOpts (...) at lib/CodeGen/CodeGenPrepare.cpp:1794</div><div>CodeGenPrepare::optimizeBlock (... at lib/CodeGen/CodeGenPrepare.cpp:6703</div><div>CodeGenPrepare::runOnFunction (...) at lib/CodeGen/CodeGenPrepare.cpp:447<br><br>(the dyn_cast is passed zero because getTerminator() on the BB returns null - because of the fact that it doesn't end with a terminator)</div><br>I'm going to revert this for now - do you think you'd need a test case from me to reproduce this failure? I can try to reduce something from the internal code that triggered this, but might take a bit of work - happy to do so if you need it but I'm hoping the rough explanation will suffice to get you to something useful faster than I can, given you've been poking around in the code already recently.<br><br>Reverted in r340794<br><br>- Dave</div><br><div class="gmail_quote"><div dir="ltr">On Tue, Aug 21, 2018 at 4:43 PM Vedant Kumar via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: vedantk<br>
Date: Tue Aug 21 16:42:38 2018<br>
New Revision: 340368<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=340368&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=340368&view=rev</a><br>
Log:<br>
[CodeGenPrepare] Scan past debug intrinsics to find select candidates (NFC)<br>
<br>
In optimizeSelectInst, when scanning for candidate selects to rewrite<br>
into branches, scan past debug intrinsics. This makes the debug-enabled<br>
and non-debug paths through optimizeSelectInst more congruent.<br>
<br>
NFC because every select is eventually visited either way.<br>
<br>
Modified:<br>
    llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp<br>
<br>
Modified: llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=340368&r1=340367&r2=340368&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=340368&r1=340367&r2=340368&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Tue Aug 21 16:42:38 2018<br>
@@ -5599,9 +5599,10 @@ bool CodeGenPrepare::optimizeSelectInst(<br>
   // Find all consecutive select instructions that share the same condition.<br>
   SmallVector<SelectInst *, 2> ASI;<br>
   ASI.push_back(SI);<br>
-  for (BasicBlock::iterator It = ++BasicBlock::iterator(SI);<br>
-       It != SI->getParent()->end(); ++It) {<br>
-    SelectInst *I = dyn_cast<SelectInst>(&*It);<br>
+  for (Instruction *NextInst = SI->getNextNonDebugInstruction();<br>
+       NextInst != SI->getParent()->getTerminator();<br>
+       NextInst = NextInst->getNextNonDebugInstruction()) {<br>
+    SelectInst *I = dyn_cast<SelectInst>(NextInst);<br>
     if (I && SI->getCondition() == I->getCondition()) {<br>
       ASI.push_back(I);<br>
     } else {<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>
</div></blockquote></div><br></div></div></blockquote></div>