[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