[PATCH] Introduce an optimisation for special case lists with large numbers of literal entries.

Peter Collingbourne peter at pcc.me.uk
Fri Jul 19 18:22:21 PDT 2013



================
Comment at: lib/Transforms/Utils/SpecialCaseList.cpp:38
@@ +37,3 @@
+  StringSet<> Strings;
+  Regex *RegEx;
+
----------------
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.

================
Comment at: lib/Transforms/Utils/SpecialCaseList.cpp:99
@@ +98,3 @@
+    // metacharacters.
+    if (Regexp.find_first_of("()^$|*+?.[]\\{}") == std::string::npos) {
+      Entries[Prefix][Category].Strings.insert(Regexp);
----------------
Alexey Samsonov wrote:
> Where does this magic list of characters come from? How can we know it's complete and modify it when needed? Looks like you need to extend Regex class for this.
It's derived from the implementation in regcomp.c which I double checked against the POSIX extended regex specification.  I've added a comment to that effect.

Switching away from POSIX extended regex syntax could arguably break more things than this list of metacharacters.  But I see the value of having the list in a central place, so I factored it out to Regex.cpp.


http://llvm-reviews.chandlerc.com/D1150



More information about the llvm-commits mailing list