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

David Greene via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 11 13:40:38 PST 2022


greened added a comment.

In D116832#3234787 <https://reviews.llvm.org/D116832#3234787>, @sebastian-ne wrote:

> The implementation looks fine to me, but I’m not sure what the goal is.

Same as the goal for other checks: make them more robust in the face of non-relevant changes.

> The only advantage I see is that if the register allocation changes, the test still passes without needing changes.

In the context of this change, yes.  I have further features to add that will allow filtering the checks so we don't *have to* check all of the output.  In that case, if code outside the checks changes (and we've said we don't care about it), then register allocation will almost certainly change and at that point you want patterns, not hard-coded names.

> But, I find the checks with regexes hard to read. The length of the regex doesn’t improve the readability either.

Granted that is an issue.

> The names for the matches are generated from the register names, so if the register allocation changed in some commit (though the test keeps passing) and someone re-generates the test later, all the match names change – possibly on a commit that is unrelated to the one that changed the register allocation.
>
> Using `<instruction name>+<counter>` like the MIR-check script does could alleviate the second issue.

That'll change too if instruction counts change.  We have this problem with `update_test_checks.py` as well.


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