[PATCH] D65948: [MCA] Add flag -show-encoding to llvm-mca.

Guillaume Chatelet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 8 06:45:04 PDT 2019


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

Nice!
Please wait for other reviewers to have a look.



================
Comment at: lib/MCA/CodeEmitter.cpp:20
+CodeEmitter::getOrCreateEncodingInfo(const MCInst &Inst) {
+  auto It = EncodingRefs.find(&Inst);
+  if (It != EncodingRefs.end())
----------------
I'm not a huge fan of using the address of `MCInst` as the key in the cache, for instance the following code would be buggy:
```
CodeEmitter CE;
MCInst Inst = GetInst();
CE.getOrCreateEncodingInfo(Inst);
Inst = GetAnotherInst();
CE.getOrCreateEncodingInfo(Inst);

```

If you still want to use the address for performance reasons you can pass by pointer instead of reference. At least this will be a red flag,


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

https://reviews.llvm.org/D65948





More information about the llvm-commits mailing list