[CGP] Avoid segmentation fault when doing PHI node simplifications
authorBjorn Pettersson <bjorn.a.pettersson@ericsson.com>
Tue, 20 Mar 2018 09:06:37 +0000 (09:06 +0000)
committerBjorn Pettersson <bjorn.a.pettersson@ericsson.com>
Tue, 20 Mar 2018 09:06:37 +0000 (09:06 +0000)
commitbf3213e48517f8d6a5123903542d807ef21c0a17
treec8973bae0a1215e343ff211a2fccc44ffc714523
parent8b8253fdc79e80f5de9aeed2201ea2d7582b729b
[CGP] Avoid segmentation fault when doing PHI node simplifications

Summary:
Made PHI node simplifiations more robust in several ways:

- Minor refactoring to let the SimplificationTracker own the
sets with new PHI/Select nodes that are introduced. This is
maybe not mapping to the original intention with the
SimplificationTracker, but IMHO it encapsulates the logic behind
those sets a little bit better.

- MatchPhiNode can sometimes populate the Matched set with
several entries, where it maps one PHI node to different candidates
for replacement. The Matched set is changed into a SmallSetVector
to make sure we get a deterministic iteration when doing
the replacements.

- As described above we may get several different replacements
for a single PHI node. The loop in MatchPhiSet that is doing
the replacements could end up calling eraseFromParent several
times for the same PHI node, resulting in segmentation faults.
This problem was supposed to be fixed in rL327250, but due to
the non-determinism(?) it only appeared to be fixed (I still
got crashes sometime when turning on/off -print-after-all etc
to get different iteration order in the DenseSets).
With this patch we follow the deterministic ordering in the
Matched set when replacing the PHI nodes. If we find a new
replacement for an already replaced PHI node we replace the
new replacement by the old replacement instead. This is quite
similar to what happened in the rl327250 patch, but here we
also recursively verify that the old replacement hasn't been
replaced already.

- It was really hard to track down the fault described above
(segementation fault due to doing eraseFromParent multiple
times for the same instruction). The fault was intermittent and
small changes in the code, or simply turning on -print-after-all
etc could make the problem go away. This was basically due to
the iteration over PhiNodesToMatch in MatchPhiSet no being
deterministic. Therefore I've changed the data structure for
the SimplificationTracker::AllPhiNodes into an SmallSetVector.
This gives a deterministic behavior.

Reviewers: skatkov, john.brawn

Reviewed By: skatkov

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D44571

llvm-svn: 327961
llvm/lib/CodeGen/CodeGenPrepare.cpp