[PATCH] D28384: Update update_test_checks to work properly with phi nodes and other fun things.

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 6 12:05:03 PST 2017


dberlin added a comment.

The benchmark is invalid since it can't possibly work doing it that way:

There is no way to do it the way you want entirely without rewriting a lot more of this code  (because we work on the scrubbed line, but can't output it), so we'd have to rewrite the function calling it

Also, this requires we capture more of the group. You are just taking group(1), which is ... not right, because it will delete all the spacing, the parens, the commas, etc, because sub expects a replacement for the entire match.
We then have to reconstitute a correct replacement by capturing all of the line.

So i think that boils down to "i can give you wrong answers  much faster" :)

A closer test would also be to just replace only starting at the match point.
The reason replace is slower is mainly because it's starting the entire string.

If you look, something like

line[:match.start(1)] + line.replace(match.group(1), whatever) + line[match.end(1):] is 6-10 times faster.

But since you seem to really want the .sub approach,  i've taken that.


https://reviews.llvm.org/D28384





More information about the llvm-commits mailing list