[PATCH] Introduce an optimisation for special case lists with large numbers of literal entries.
Alexey Samsonov
samsonov at google.com
Fri Aug 2 05:11:44 PDT 2013
Sorry for the delay...
================
Comment at: include/llvm/Support/Regex.h:82
@@ +81,3 @@
+ /// matches itself and only itself.
+ static bool isLiteral(StringRef Str);
+
----------------
Okay. I think this can get approval from Regex author and be submitted separately (likely, with a unittest).
================
Comment at: lib/Transforms/Utils/SpecialCaseList.cpp:38
@@ +37,3 @@
+ StringSet<> Strings;
+ Regex *RegEx;
+
----------------
Peter Collingbourne wrote:
> Alexey Samsonov wrote:
> > Consider:
> > a) using OwningPtr<Regex> instead of RegEx*. In this case you wouldn't need SpecialCaseList destructor and Entry ctor.
> > b) making this a class with private data. You need only: addString(), setRegex() and match() methods.
> >
> > I would also appreciate a comment about the intent behind this.
> > a) using OwningPtr<Regex> instead of RegEx*. In this case you wouldn't need SpecialCaseList destructor and Entry ctor.
>
> This won't work because StringMap requires its element type to be copyable and assignable.
>
> > b) making this a class with private data. You need only: addString(), setRegex() and match() methods.
>
> I'd prefer not to do this, as Entry is only an implementation detail of SpecialCaseList.
>
> > I would also appreciate a comment about the intent behind this.
>
> Done.
Why not use
~Entry() { delete RegEx; }
then, instead of iterating over two-level StringMap then?
http://llvm-reviews.chandlerc.com/D1150
More information about the llvm-commits
mailing list