[PATCH] D77999: Reimplement mlir::Identifier to be a wrapper around 'StringMapEntry*' instead of a wrapper around a 'const char*'. This makes it so strref() can be computed without calling strlen, which is more efficient and less error-prone. While here...

River Riddle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 13 10:45:01 PDT 2020


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


================
Comment at: mlir/include/mlir/IR/Identifier.h:83
 inline bool operator==(Identifier lhs, Identifier rhs) {
   return lhs.data() == rhs.data();
 }
----------------
rriddle wrote:
> lattner wrote:
> > rriddle wrote:
> > > rriddle wrote:
> > > > Can you also fix these while you are at it? These should really just be inline operators that compare the entry.
> > > (By inline I mean in the class)
> > I changed these to compare the entry, and moved the Id/Id versions inline.  However, we can't move the StringRef versions inline splitting the equality operators into two groups.
> > 
> > I think both are fine, but please give me an a +1 that this is what you wanted.
> > 
> > Incidentally, the is() method should go away now.  I can take care of that cleanup in a separate patch.
> Yeah, that's what I meant. +1 thanks.
(i.e. what you have looks good to me. I only meant the Identifier, Identifier ones)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77999





More information about the llvm-commits mailing list