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

[Daniel Berlin via llvm-commits]

We can do better, but not in this way (see my other email, this will
generate wrong answers) , and it's IMHO not worth it.

You can avoid doing the replacement directly by using the match start and
end, but if you do, we either have to mess with the loop quite a bit.
Even if we stop using finditer (because we are replacing the string it is
using as it goes, and it does not keep up to date) and use match directly,
we have to offset where it starts looking for matches by the amount of text
we've added by converting to var defs/uses, etc, or risk false matches.


None of this is worth it compared to keeping it simple like I did, imho.


On Fri, Jan 6, 2017 at 9:26 AM, bryant via Phabricator <
reviews at reviews.llvm.org> wrote:

> bryant added inline comments.
>
>
> ================
> Comment at: utils/update_test_checks.py:176
> +      if match.group(1) in vars_seen:
> +        line = line.replace('%' + match.group(1),
> get_value_use(match.group(1)), 1)
> +        continue
> ----------------
> bryant wrote:
> > You're limiting the number of replaced uses to 1. Can a line have more
> than one use?
> Just to clarify, what I meant is that you can do a lot better than
> manually replacing one at a time:
>
> ```
> def genericize_check_lines(lines):
>     def ssaify(match):
>         v = match.group(1)
>         if v in var_seen:
>             rv = get_value_use(v)
>         else:
>             vars_seen.add('%' + v)
>             rv = get_value_definition(v)
>         return rv
>     for line in lines:
>         scrubbed_line = ...
>         new_ine = IRVALUE_RE.sub(ssaify, scrubbed_line)
>     ...
> ```
>
> I suspect that underneath, `_sre` is building the replacement string
> incrementally, so it won't be quadratic.
>
>
> https://reviews.llvm.org/D28384
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170106/ea026c74/attachment.html>