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

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added a comment.

In D116832#3254716 <https://reviews.llvm.org/D116832#3254716>, @greened wrote:

> In D116832#3253630 <https://reviews.llvm.org/D116832#3253630>, @pengfei wrote:
>
>> A patch that only exchanges the order of the first add to
>>
>>   ; CHECK-NEXT:    add %R0, %R2
>>   ; CHECK-NEXT:    add %R0, %R1
>>
>> Then, the other use and def of %R1 <https://reviews.llvm.org/diffusion/L/> and %R2 <https://reviews.llvm.org/source/clang-tools-extra/> will remain unchanged.
>> But if you are numbering the register by order, all the other %R1 <https://reviews.llvm.org/diffusion/L/> and %R2 <https://reviews.llvm.org/source/clang-tools-extra/> will be changed while the expected change won't be shown. This is rather confusing.
>
> Fair enough.  Again, I would ask, what is the goal of the test.  If it's testing scheduling, then don't scrub register names.  If it's not testing scheduling, then why do we care if the scheduling change isn't caught by the test checks?

What I mainly wanted to demonstrate is there exists such a test generated for other purpose. For a scheduling patch, the unexpected changes of other use and def of %R1 <https://reviews.llvm.org/diffusion/L/> and %R2 <https://reviews.llvm.org/source/clang-tools-extra/> on an unrelated test are confusing.

  ; CHECK-NEXT:    add %[[GPR_1:(%r[0-9]+)]], %[[GPR_2:(%r[0-9]+)]] <== These two lines won't be changed, that's fine.
  ; CHECK-NEXT:    add %[[GPR_1:(%r[0-9]+)]], %[[GPR_3:(%r[0-9]+)]]
  ; ... ...
  ; CHECK-NEXT:    other_use_def %[[GPR_2]] <== These lines will be changed, that's bad.
  ; CHECK-NEXT:    other_use_def %[[GPR_3]] <== Same here.



> A simple uniquing counter will result in more differences than ideal.  I could imagine any number of schemes to increase precision but my sense is that it's not worth it.  If the user is also filtering output to create smaller test checks, then the uniquing tends to work better IME.
>
> This is one tool in the toolbox.  It's not meant for every situation.

So I'm not arguing don't use this tool for number sensitive patches. I'm arguing any test cases generated in this way may confuse future number sensitive patches.
I agree filtering out unrelated code is a good idea. Then why we still need this patch if we have already filtered them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116832