[PATCH] D86968: [IRSim] Adding IR Instruction Mapper
Jessica Paquette via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 16 15:57:08 PDT 2020
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
More information about the llvm-commits
mailing list