[PATCH] D52540: Fix an ordering bug in the scalarizer.

Thu Sep 27 06:23:29 PDT 2018

sheredom added a comment.

In https://reviews.llvm.org/D52540#1247810, @uabelho wrote:

> dstenb described a possible fix in a comment of PR28911 and we've been using that fix for a long time now for our
>  out-of-tree target without problems.
>
> Perhaps that fix is a hack and using RPOT is the proper way to deal with this, I've no idea. I just wanted to point out the
>  possibility.

I took a look at dstenb's approach and it is definitely not a hack - its just solving this issue from a slightly different viewpoint. Their approach just ensure thats the lists of values are never accidentally invalidated, whereas mine forces us to walk the BB graph in order such that we should never generate invalid lists in the first place.

> Anyway, I applied the fix in this patch on our local tree and so far so good so I don't have any objections, but I don't
>  really know the code enough to say it's the right thing to do.

Great - glad it works for you!

Should I incorporate the test case from the bugzilla into this commit? Given that its a slightly different failure case that is also fixed by my change, seems like its worth ensuring it doesn't accidentally regress again.

Repository:
  rL LLVM

https://reviews.llvm.org/D52540