[PATCH] D86973: [IRSim] Adding wrapper pass for IRSimilarityIdentfier

[Andrew Litteken via Phabricator via llvm-commits]

AndrewLitteken added inline comments.


================
Comment at: llvm/lib/Analysis/IRSimilarityIdentifier.cpp:685
+  Optional<SimilarityGroupList> &SimilarityCandidatesOpt = IRSI.getSimilarity();
+  assert(SimilarityCandidatesOpt.hasValue() && "SimilarityCandidates is None!");
+  SimilarityGroupList &SimilarityCandidates =
----------------
jroelofs wrote:
> 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.
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86973/new/

https://reviews.llvm.org/D86973