This is an archive of the discontinued LLVM Phabricator instance.

[IRSim] Adding wrapper pass for IRSimilarityIdentfier
ClosedPublic

Authored by AndrewLitteken on Sep 1 2020, 1:19 PM.

Details

Summary

This introduces an analysis pass that wraps IRSimilarityIdentifier, and adds a printer pass to examine in what function similarities are being found.
Test for what the printer pass can find are in test/Analysis/IRSimilarityIdentifier.

Diff Detail

Event Timeline

AndrewLitteken created this revision.Sep 1 2020, 1:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 1 2020, 1:19 PM
AndrewLitteken added a reviewer: paquette.

Updating for clang-format.

jroelofs accepted this revision.Sep 15 2020, 9:45 AM
jroelofs added a subscriber: jroelofs.

LGTM

This revision is now accepted and ready to land.Sep 15 2020, 9:45 AM

Updating for the new Optional in IRSimilarityIdentifier.

jroelofs added inline comments.Sep 17 2020, 1:48 PM
llvm/lib/Analysis/IRSimilarityIdentifier.cpp
680

The accessors for Optional do this check for you, so you can just "dereference" it with * or -> as needed without worrying about adding your own assertion on it.

If it's expected that IRSI will always have this populated once it has been constructed, then maybe getSimilarity() ought to be the one to unwrap the Optional.

AndrewLitteken added inline comments.Sep 17 2020, 1:53 PM
llvm/lib/Analysis/IRSimilarityIdentifier.cpp
680

Well findSimilarity needs to have been run, or the constructor where a module has been passed to the IRSI. So, it's possible that you can construct IRSI where the SimilarityCandidates are None.

I'm not sure if that changes what you're proposing. I think it could still make sense to put the derefence in getSimilarity since you shouldn't be calling it unless you've populated the IRSI with a module already.

Updating optional handling

jroelofs added inline comments.Sep 18 2020, 12:01 PM
llvm/lib/Analysis/IRSimilarityIdentifier.cpp
680

I think it could still make sense to put the derefence in getSimilarity

agreed

684

So maybe this is just:

for (std::vector<IRSimilarityCandidate> &CandVec : IRSI.getSimilarity()) {

or

for (std::vector<IRSimilarityCandidate> &CandVec : *IRSI.getSimilarity()) {

depending on which way you decide to go w/ moving that dereference.