[PATCH] D58098: [IR] Add Use::moveToFrontOfUseList() method

Roman Tereshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 19 16:52:04 PST 2019


rtereshin marked an inline comment as done.
rtereshin added inline comments.


================
Comment at: include/llvm/IR/Use.h:132
+  /// close to it) of the use list, reducing the amortized cost of such lookups.
+  inline void moveToFrontOfUseList();
+
----------------
dexonsmith wrote:
> I'm curious if there's really a compelling reason to define this inline in Value.h.  It doesn't seem like it will be used very frequently (and I assume most vendors compile with some form of LTO anyway), so without profiling data I'd rather this just be in Use.cpp.
Ah, I missed that one.

I didn't profile anything with LTO on, but with a regular -O3 build the in Release the unittest included here performed ever so slightly better (with larger repetition counts), I don't quite remember the exact numbers already, but I think it was a couple of percent.

Also, `Use::set` is defined like this and I liked the idea of putting them next to each other because they are very similar in implementation. `moveToFrontOfUseList` is practically a specialization of `set` for the current value of the use.

If that doesn't change your mind, I'll put it in cpp, I don't have a strong opinion about this.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58098





More information about the llvm-commits mailing list