[PATCH] D106989: [IRSim] Finding Branch Similarity

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/IROutliner.cpp:1361
+  IRInstructionDataList::iterator IDIt = std::next(ID.getIterator());
+  Instruction *NextIDInst = IDIt->Inst;
+  Instruction *NextInst = nullptr;
----------------
This name is kind of confusing considering the other `Instruction` is named `NextInst`?


================
Comment at: llvm/lib/Transforms/IPO/IROutliner.cpp:1363
+  Instruction *NextInst = nullptr;
+  if (!ID.Inst->isTerminator())
+    NextInst = ID.Inst->getNextNonDebugInstruction();
----------------
Just use `NextIDInst` here?


================
Comment at: llvm/lib/Transforms/IPO/IROutliner.cpp:1365
+    NextInst = ID.Inst->getNextNonDebugInstruction();
+  else if (IDIt->Inst != nullptr)
+    NextInst =
----------------
Same here?


================
Comment at: llvm/test/Transforms/IROutliner/region-end-of-module.ll:4
+
+; This test checks that we do not faul when there is a similarity group with
+; an ending instruction that is also the end of the module.
----------------



================
Comment at: llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp:44
     std::vector<std::vector<IRSimilarityCandidate>> &SimilarityCandidates) {
-  IRSimilarityIdentifier Identifier;
+  IRSimilarityIdentifier Identifier(false);
   SimilarityCandidates = Identifier.findSimilarity(M);
----------------
Add comment saying what the false is describing?

```
Identifier(/*blah = */false);
```


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

https://reviews.llvm.org/D106989