[PATCH] D60600: [InstCombine] Fix a vector-of-pointers instcombine undef bug.

Neil Henning via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 1 05:41:46 PDT 2019


sheredom added a comment.

In D60600#1485496 <https://reviews.llvm.org/D60600#1485496>, @reames wrote:

> In D60600#1475212 <https://reviews.llvm.org/D60600#1475212>, @sheredom wrote:
>
> > In D60600#1470778 <https://reviews.llvm.org/D60600#1470778>, @reames wrote:
> >
> > > I think you might be overcomplicating this.  I'd suggest the following:
> > >
> > > 1. scan all index types for a gep
> > > 2. run current code, but if any operand was gep, skip the recursive call on the operand
> > >
> > >   It's slightly less powerful, but correct, and easy.  And frankly, optimize geps of struct types is probably rare enough we don't care.
> > >
> > >   p.s. Agreed on the location of test file, please add to existing tests.
> >
> >
> > I don't really understand what your suggestion is here sorry - let me try and rephrase the suggestion and see if we agree?
>
>
> I submitted a version along what I proposed as https://reviews.llvm.org/rL359633.  Please confirm that fixes your crash.  If you want to work on a better version of the fix, I'm happy to review, but I wanted to make sure the crash was resolved.


I can confirm your fix does fix my crash.

I guess by 'better version of the fix' you mean relaxing the langref and dealing with the fallout? The only intermediate fix would be what this review originally proposed.

Is there any way to get the history of why the lang ref had the struct-index restriction it did? My only notion is that it is to ensure that each element of the vector-of-pointers had the same 'depth' of GEP.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60600/new/

https://reviews.llvm.org/D60600





More information about the llvm-commits mailing list