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

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:108
+  /// The legality of the wrapped instruction.
+  bool Legal = true;
+
----------------
Is this related to `InstrType`?


================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:227
+/// MachineInstrs, rather than IRInstructionData, the wrapper for an
+/// Instruction.
+struct IRInstructionMapper {
----------------
This is pretty much the same as the MachineOutliner's instruction mapper. Can you add a TODO saying that the mappers should be merged?


================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:231
+  ///
+  /// Set to -3 for compatibility with \p DenseMapInfo<unsigned>.
+  unsigned IllegalInstrNumber = static_cast<unsigned>(-3);
----------------
Don't need `\p` here


================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:295
+    // changed.
+    assert(DenseMapInfo<unsigned>::getEmptyKey() == (unsigned)-1 &&
+           "DenseMapInfo<unsigned>'s empty key isn't -1!");
----------------
Prefer `static_cast` over C-style casts for greppability.


================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:297
+           "DenseMapInfo<unsigned>'s empty key isn't -1!");
+    assert(DenseMapInfo<unsigned>::getTombstoneKey() == (unsigned)-2 &&
+           "DenseMapInfo<unsigned>'s tombstone key isn't -2!");
----------------
Prefer `static_cast` over C-style casts for greppability.


================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:301
+
+  IRInstructionMapper(BumpPtrAllocator *IDA)
+      : InstDataAllocator(IDA){
----------------
Do you need both constructors?

Would something like the suggested edit above work?


================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:363
+#endif // LLVM_ANALYSIS_IRSIMILARITYIDENTIFIER_H
\ No newline at end of file

----------------
add newline


================
Comment at: llvm/lib/Analysis/IRSimilarityIdentifier.cpp:162
+}
\ No newline at end of file

----------------
add newline


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

https://reviews.llvm.org/D86968