[llvm] r340368 - [CodeGenPrepare] Scan past debug intrinsics to find select candidates (NFC)

[David Blaikie via llvm-commits]

No worries at all - thanks for looking into it!

On Tue., 28 Aug. 2018, 10:58 am Vedant Kumar, <vsk at apple.com> wrote:

> Thanks for reverting Dave. Sorry for the breakage. This should be enough
> information for me to revise the patch & add a regression test.
>
> vedant
>
> On Aug 27, 2018, at 5:56 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
> Looks like this is causing a crash - the pass later assumes that all the
> select instructions discovered are contiguous.
>
> 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 i1 %495, label ..., label ..., !dbg !251, !prof !252
>     call void @llvm.dbg.value(metadata float ..., metadata !152, metadata
> !DIExpression()), !dbg !241
>
> (specifically, the backtrace is:
>
> llvm::dyn_cast<llvm::ReturnInst, llvm::TerminatorInst> (Val=0x0) at
> include/llvm/Support/Casting.h:334
> CodeGenPrepare::dupRetToEnableTailCallOpts (...) at
> lib/CodeGen/CodeGenPrepare.cpp:1794
> CodeGenPrepare::optimizeBlock (... at lib/CodeGen/CodeGenPrepare.cpp:6703
> CodeGenPrepare::runOnFunction (...) at lib/CodeGen/CodeGenPrepare.cpp:447
>
> (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)
>
> 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.
>
> Reverted in r340794
>
> - Dave
>
> On Tue, Aug 21, 2018 at 4:43 PM Vedant Kumar via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: vedantk
>> Date: Tue Aug 21 16:42:38 2018
>> New Revision: 340368
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=340368&view=rev
>> Log:
>> [CodeGenPrepare] Scan past debug intrinsics to find select candidates
>> (NFC)
>>
>> In optimizeSelectInst, when scanning for candidate selects to rewrite
>> into branches, scan past debug intrinsics. This makes the debug-enabled
>> and non-debug paths through optimizeSelectInst more congruent.
>>
>> NFC because every select is eventually visited either way.
>>
>> Modified:
>>     llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
>>
>> Modified: llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=340368&r1=340367&r2=340368&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Tue Aug 21 16:42:38 2018
>> @@ -5599,9 +5599,10 @@ bool CodeGenPrepare::optimizeSelectInst(
>>    // Find all consecutive select instructions that share the same
>> condition.
>>    SmallVector<SelectInst *, 2> ASI;
>>    ASI.push_back(SI);
>> -  for (BasicBlock::iterator It = ++BasicBlock::iterator(SI);
>> -       It != SI->getParent()->end(); ++It) {
>> -    SelectInst *I = dyn_cast<SelectInst>(&*It);
>> +  for (Instruction *NextInst = SI->getNextNonDebugInstruction();
>> +       NextInst != SI->getParent()->getTerminator();
>> +       NextInst = NextInst->getNextNonDebugInstruction()) {
>> +    SelectInst *I = dyn_cast<SelectInst>(NextInst);
>>      if (I && SI->getCondition() == I->getCondition()) {
>>        ASI.push_back(I);
>>      } else {
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180828/3c8a8aaf/attachment.html>