[llvm-commits] PATCH: Make instsimplify survive degenerate, unreachable code formations

Chandler Carruth chandlerc at gmail.com
Fri Mar 23 10:36:27 PDT 2012


Updated patch with doxygen fixes and one goof where we returned true and
shouldn't have.

On Fri, Mar 23, 2012 at 10:26 AM, Chandler Carruth <chandlerc at gmail.com>wrote:

> Hello!
>
> The current instsimplify ReplaceAndSimplifyAllUses function has a bug that
> impacts its usage from inside the inliner, or in other contexts where it
> may try to simplify code inside of an unreachable block. It checks
> initially that From != To, but it then uses a WeakVH to keep track of both
> From and To. The result is that mutually recursive values can, in some very
> strange situations, end up replacing the From instruction with the To
> instruction deep inside of a recursion. When popping back out, the WeakVH
> tracks this, and we end up in an infinite loop trying to simplify an
> instruction with >0 uses into itself.
>
> It appears the code had an incorrect assumption about how WeakVH works --
> it expected the value to become null on either RAUW or deletion, when in
> fact WeakVH follows RAUW and becomes null on deletion.
>
> That said, I think there is a simpler algorithm for performing the
> recursive simplification that isn't actually recursive. Also, we can expose
> an interface that more naturally represents what most clients want: take an
> instruction, recursively simplify it, tell us if any changes were made.
> Using the worklist also doesn't suffer from the problem of VH migration.
>
> It also plays a bit nicer with other users who have a VH, they get an RAUW
> callback when their use is replaced rather than us iterating over the uses
> and stomping on them individually.
>
> Thoughts? I'm still adding doxygen comments, but wanted feedback on the
> implementation sooner.
>
> I don't have any way to test this really. I found this when I hit an
> infloop in the test suite when using instruction simplify in the inliner.
> The only code formation that triggers it is a deferred-phi-simplification
> of phi nodes in an unreachable chain of blocks... I don't know of any other
> path to this formation, nor even a way to reduce this into a nice
> regression test for the inliner changes. =/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120323/74382ace/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: instsimplify-worklist.diff
Type: application/octet-stream
Size: 10539 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120323/74382ace/attachment.obj>


More information about the llvm-commits mailing list