[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 19:10:13 PST 2022


greened added a comment.

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

> Agreed with Simon, this is a disaster for doing code review.

That seems a bit extreme.

> I believe almost lit tests won't be affected by RA changes given they are concise. For test cases that already complicated, the regexes make the readablity even worse.

I disagree that lit test cases are concise.  We have a huge issue with codegen churn causing pages of testcase changes in proposed merges and very little review is done on them.  I'm trying to get us to a place where we have less of that.  Filtering out trivial changes to tests in reviews can be a big win.

> According to my experience, jumping between xmm0 and xmm1 is much more tolerable that regexes. And yes, it's easy to hide unnoticeable flaws.

How so?  I'm struggling to think of a time a regexp hid a real problem.  Can you explain a bit more?


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