[PATCH] D158675: [NFC][AsmPrinter] Refactor DbgVariable as a std::variant

[Felipe de Azevedo Piovezan via Phabricator via llvm-commits]

fdeazeve added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h:149
+struct MMILoc {
+  mutable SmallVector<FrameIndexExpr, 1> FrameIndexExprs;
+};
----------------
scott.linder wrote:
> fdeazeve wrote:
> > Not for this patch, but it really bothers me that this is mutable. The original rationale must have been to allow sorting when we call the "get" method, but still...
> It is almost spooky how you've pointed out the same things I toyed with while working on the patches originally!
> 
> I had a patch in the middle somewhere which just exposed the need to sort in the interface, but scrapped it early on. I think your `set` idea makes a lot of sense and I'll follow up with a patch for that as well.
One thing to consider here: sorting (either in the current vector approach or through the internal impl of std::set) require calling `Expr->getFragmentInfo()->OffsetInBits` as many time as comparison operations are performed, and this is possibly done multiple times for the same expression without any caching.

Something to keep in mind as a possible improvement as well (perhaps computing & storing the offset in advance)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158675