This is an archive of the discontinued LLVM Phabricator instance.

[CFLAA] Change FunctionHandle to be common to Steensgaard's and Andersens'
ClosedPublic

Authored by davide on Jun 26 2017, 12:18 PM.

Details

Summary

In order to fix https://bugs.llvm.org/show_bug.cgi?id=33339 we need to keep CallBackVH for instructions inserted in the stratified sets so that we can remove when, e.g. instcombine or any other transform remove the instruction.

When looking at fixing this I noticed that Andersens' and Steensgaard's analyses end up duplicating a considerable amount of code, including the cache. This is a first step towards the unification of what's common to the two analyses.

Diff Detail

Event Timeline

davide created this revision.Jun 26 2017, 12:18 PM

LGTM with two nits. Thanks!

include/llvm/Analysis/CFLAliasAnalysisUtils.h
10

Nit: Would "These are the utilities/helpers [...]" be better here?

lib/Analysis/CFLAndersAliasAnalysis.cpp
792

Nit: While we're in the area, can this and the other push_front we're touching use emplace_front(const_cast<Function *>(&Fn), this) instead? That would let us skip moving this, which seems to involve nontrivial operations.

This revision is now accepted and ready to land.Jun 26 2017, 2:22 PM
davide added inline comments.Jun 26 2017, 4:53 PM
include/llvm/Analysis/CFLAliasAnalysisUtils.h
10

Yes, I suck at writing proper english.

lib/Analysis/CFLAndersAliasAnalysis.cpp
792

Indeed. I also dislike a lot templating explicitly when there's no need. Let me try that.

This revision was automatically updated to reflect the committed changes.