[llvm] r334317 - [InstCombine] Skip dbg.value(s) when looking at stack{save, restore}.

[Davide Italiano via llvm-commits]

On Fri, Jun 8, 2018 at 4:02 PM, Vedant Kumar <vsk at apple.com> wrote:
> Hi Davide,
>
> A question inline --
>
>> On Jun 8, 2018, at 1:42 PM, Davide Italiano via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: davide
>> Date: Fri Jun  8 13:42:36 2018
>> New Revision: 334317
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=334317&view=rev
>> Log:
>> [InstCombine] Skip dbg.value(s) when looking at stack{save,restore}.
>>
>> Fixes PR37713.
>>
>> Added:
>>    llvm/trunk/test/Transforms/InstCombine/stacksave-debuginfo.ll
>> Modified:
>>    llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=334317&r1=334316&r2=334317&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Fri Jun  8 13:42:36 2018
>> @@ -3521,8 +3521,15 @@ Instruction *InstCombiner::visitCallInst
>>     // happen when variable allocas are DCE'd.
>>     if (IntrinsicInst *SS = dyn_cast<IntrinsicInst>(II->getArgOperand(0))) {
>>       if (SS->getIntrinsicID() == Intrinsic::stacksave) {
>> -        if (&*++SS->getIterator() == II)
>> +        // Skip over debug info instructions.
>> +        // FIXME: This should be an utility in Instruction.h
>> +        auto It = SS->getIterator();
>> +        It++;
>> +        while (isa<DbgInfoIntrinsic>(*It))
>
> ^ Can you dereference the sentinel node in this while loop's condition?
>

I don't think so, unless the basic block is malformed. Your iterator
points to a `stacksave`.
At that point you are guaranteed to have at least another instruction,
i.e. the terminator of the block (otherwise the block is malformed),
which doesn't satisfy the `isa<DbgInfoInstrinsic>`) condition.
Does this make sense to you?

Thanks,

--
Davide