Page MenuHomePhabricator

[NewGVN] Minor cleanups
AbandonedPublic

Authored by nikic on May 17 2018, 4:49 AM.

Details

Reviewers
dberlin
Summary

This patch implements a number of minor cleanups in NewGVN. If desired I can split this up further.

  • In setMemoryClass(), assert that the memory access is already in MemoryAccessToClass. Previously the code simply did nothing if it was not in the map, which is confusing.
  • Add a missing deleteExpression() in PHI Evaluation.
  • Simplify handling of the false edge of branch predicates. We can handle this the same way as the true edge with an inverted predicate. This is both more general and simpler than the previous code.
  • Fix a copy&paste mistake in addPredicateUsers which caused assumes() to not be handled. I think that due to the specific circumstances of the use, this did not cause issues.
  • Simplify SwitchInst edge processing. Don't create an unused SwitchEdges map and handle default+cases the same. This code was probably doing something more prior to the introduction of PredicateInfo and this got left behind.
  • Fix some comments.

Diff Detail

Event Timeline

nikic created this revision.May 17 2018, 4:49 AM
fhahn added a subscriber: fhahn.May 17 2018, 5:13 AM
fhahn added inline comments.
lib/Transforms/Scalar/NewGVN.cpp
1930–1939

Nit: no braces for one statements blocks.

nikic updated this revision to Diff 147302.May 17 2018, 6:03 AM

Address nit.

dberlin added subscribers: junbuml, nikic.EditedMay 17 2018, 8:18 AM
Simplify handling of the false edge of branch predicates. We can handle this the same way as the true edge with an inverted predicate. This is both more general and simpler than the previous code.

I swear we have had an issue with an edge case that made this not true in
the past, which is what led to the current code.

If you've actually 3 stage bootstrapped clang+llvm and tested this, i'm
okay with it for now.

nikic abandoned this revision.Jun 22 2019, 3:29 AM

Applied trivial parts of this in rL364129 and rL364130. Abandoning the rest as I don't do clang bootstraps.