[PATCH] D38200: [GISel]: Process new insts to legalize in the order they were created

Aditya Nandakumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 10 10:47:15 PDT 2017


aditya_nandakumar added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/Legalizer.cpp:55
 
+struct WorkListType {
+  WorkListType() = default;
----------------
kristof.beyls wrote:
> I don't think I've seen the naming convention of ending type names with "Type" in the LLVM code base - but I also by far haven't looked at all the LLVM code.
> Maybe "struct WorkList" would be better.
> 
> I find it unfortunate that we would need to create a custom container here for this single specific use case. Is there really not a pre-existing container that is good enough? My guess is that this WorkListType's advantage over a deque is only that it can check quickly if an element is present in the container. Is that really such a frequent operation that that must be fast, but removing that element if present doesn't have to be faster than deque.erase?
Thanks for the naming convention tip. "struct Worklist" does sound better.
I wanted a SetVector's equivalent which lets me do push_back and pop_front . As you said, the advantage over just the deque is quickly lookup if an element already exists. That said, I haven't measured how critical this is or how frequently this operation occurs.



Repository:
  rL LLVM

https://reviews.llvm.org/D38200





More information about the llvm-commits mailing list