llvm dev - Mar 2021 - Invalid llvm-ir and unreachable code

Hi Roman,

[..]
> >
> > - the problematic code construct consists of a cycle of 'dead'
basic blocks
> that conditionally jumps into live blocks.
> >
> >   That jump introduce a connection of a dead self-referring
'getelementptr'
> to live code through a PHI node.
> >
> > - during SROA, the scoped noalias analysis (full restrict version)
does an
> unbounded (MaxLookup == 0) 'getUnderlyingObject' call on a pointer.
> >
> > - 'getUnderlyingObject' also follows the path to the dead
blocks and ends up
> in the self-referring  'getelementpr'.
> >
> >
> >
> > Does this mean that 'getUnderlyingObject' should also be
prepared to be
> handling 'invalid instructions' in 'dead code' ?
> I've just adjusted that function in
36f1c3db66f7268ea3183bcf0bbf05b3e1c570b4
> to assert that no cycles are encountered instead of endlessly looping.
> 
> But i *think* the problem is in your SROA changes.
> Can you link me to the particular patch in question?
This is the patch that triggers the unbounded lookup: 
https://reviews.llvm.org/D68507
(llvm/lib/Analysis/ScopedNoAliasAA.cpp, line 269)

I checked again the endless loop: It happens _right after SROA_, in the
MemorySSA pass.

Apparently, your extra check also triggers cases without my patches ? A
self-reference is
likely a mild version of an 'invalid instruction' but what other
constructs can we encounter
in dead blocks ?

I am wondering if we should allow (analysis) passes to follow code into dead
blocks at all ?

Greetings,

Jeroen

> On Mar 15, 2021, at 09:53, Jeroen Dobbelaere via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hi Roman,
> 
> [..]
>>> 
>>> - the problematic code construct consists of a cycle of
'dead' basic blocks
>> that conditionally jumps into live blocks.
>>> 
>>>  That jump introduce a connection of a dead self-referring
'getelementptr'
>> to live code through a PHI node.
>>> 
>>> - during SROA, the scoped noalias analysis (full restrict version)
does an
>> unbounded (MaxLookup == 0) 'getUnderlyingObject' call on a
pointer.
>>> 
>>> - 'getUnderlyingObject' also follows the path to the dead
blocks and ends up
>> in the self-referring  'getelementpr'.
>>> 
>>> 
>>> 
>>> Does this mean that 'getUnderlyingObject' should also be
prepared to be
>> handling 'invalid instructions' in 'dead code' ?
>> I've just adjusted that function in
36f1c3db66f7268ea3183bcf0bbf05b3e1c570b4
>> to assert that no cycles are encountered instead of endlessly looping.
>> 
>> But i *think* the problem is in your SROA changes.
>> Can you link me to the particular patch in question?
> 
> This is the patch that triggers the unbounded lookup: 
https://reviews.llvm.org/D68507
> (llvm/lib/Analysis/ScopedNoAliasAA.cpp, line 269)
> 
> I checked again the endless loop: It happens _right after SROA_, in the
MemorySSA pass.
> 
Can you isolate the failure to just a run of MemorySSA? If so, it sounds like it
would be a problem in your linked patch. I guess it would be good to know
exactly where an infinite loop is triggered (and why).

On Mon, Mar 15, 2021 at 12:53 PM Jeroen Dobbelaere
<Jeroen.Dobbelaere at synopsys.com> wrote:
>
> Hi Roman,
>
> [..]
> > >
> > > - the problematic code construct consists of a cycle of
'dead' basic blocks
> > that conditionally jumps into live blocks.
> > >
> > >   That jump introduce a connection of a dead self-referring
'getelementptr'
> > to live code through a PHI node.
> > >
> > > - during SROA, the scoped noalias analysis (full restrict
version) does an
> > unbounded (MaxLookup == 0) 'getUnderlyingObject' call on a
pointer.
> > >
> > > - 'getUnderlyingObject' also follows the path to the dead
blocks and ends up
> > in the self-referring  'getelementpr'.
> > >
> > >
> > >
> > > Does this mean that 'getUnderlyingObject' should also be
prepared to be
> > handling 'invalid instructions' in 'dead code' ?
> > I've just adjusted that function in
36f1c3db66f7268ea3183bcf0bbf05b3e1c570b4
> > to assert that no cycles are encountered instead of endlessly looping.
> >
> > But i *think* the problem is in your SROA changes.
> > Can you link me to the particular patch in question?
>
> This is the patch that triggers the unbounded lookup: 
https://reviews.llvm.org/D68507
> (llvm/lib/Analysis/ScopedNoAliasAA.cpp, line 269)
>
> I checked again the endless loop: It happens _right after SROA_, in the
MemorySSA pass.
> Apparently, your extra check also triggers cases without my patches ? A
self-reference is
> likely a mild version of an 'invalid instruction' but what other
constructs can we encounter
> in dead blocks ?
Yeah, i fooled myself there.
> I am wondering if we should allow (analysis) passes to follow code into
dead blocks at all ?
I wouldn't think so. It's a waste of time i would say.
> Greetings,
>
> Jeroen
>