[PATCH] D116832: [UpdateLLCTestChecks] Allow replacing register names with variables

David Greene via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 9 15:48:31 PST 2022


greened added a comment.

In D116832#3303166 <https://reviews.llvm.org/D116832#3303166>, @pengfei wrote:

> I think my concerns still exist, even though you provide one more option. The author selects between cannot foresee what in their tests will be affected by future commit. And the `--scrub-reg deps` loses too much information.

There are use-cases for scrubbing names and deps.  The test author should know what they are testing.  If the author can't understand whether or not register names/deps are important, perhaps it's not a good test.  IME this kind of scrubbing really helps for creating focused tests.  This isn't an option to use blindly, the author should understand what they're doing.

> Besides, comparing the introdued complexity in `asm.py` and the (probably) limited benefit, it's not a good deal to me.

If people want to keep generating test checks from all of the asm output, perhaps there's limited benefit.  But that means we just don't care about creating focused tests.  That to me is not a great outcome.

> So back to @lebedev.ri 's question, if this is mainly for downstream use, it's better to keep it downstream. Otherwise, please give a scope which in tree tests you want to apply it to, so that we can judge the value based on that.

Now that D119368 <https://reviews.llvm.org/D119368> is posted for review, it provides another motivating case.  Most likely folks will want to scrub the temporary `SDNode` names from the isel output.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116832/new/

https://reviews.llvm.org/D116832



More information about the llvm-commits mailing list