[PATCH] D86968: [IRSim] Adding IR Instruction Mapper

[Jessica Paquette via Phabricator via llvm-commits]

paquette accepted this revision.
paquette added a comment.
This revision is now accepted and ready to land.

I pretty much only have comment nits at this point, so I think this LGTM.



================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:17
+//
+// %1 = add i32 %a, 10
+// %2 = add i32 %a, %1
----------------
Can you add `\code` and `\endcode` around these?


================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:52
+///
+/// Legal is an Instruction that is valid, and should be considered when looking
+/// for similarity between Instructions.
----------------
These comments can be slightly more succinct. Talking about validity doesn't add any information here. It would also be good to be clearer in what role these play.

> Legal Instructions are considered when looking for similarity between Instructions.
> Illegal Instructions cannot be considered when looking for similarity between Instructions. They act as boundaries between similarity regions.
> Invisible Instructions are skipped over during similarity analysis.

etc


================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:88
+///
+
+/// This hashing scheme will be used to represent the program as a very long
----------------
Missing `///`?


================
Comment at: llvm/lib/Analysis/IRSimilarityIdentifier.cpp:26
+  // Here we collect the operands to determine whether
+  // the structure of the operand use matches between two different candidates.
+  for (Use &OI : I.operands())
----------------
The operand use?


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

https://reviews.llvm.org/D86968