Page MenuHomePhabricator

[Utils] Allow "on-the-fly" argument changes for update_test_check scripts
Needs RevisionPublic

Authored by jdoerfert on Oct 31 2019, 8:53 PM.

Details

Summary

Update test scripts were limited because they performed a single action
on the entire file and if that action was controlled by arguments, like
the one introduced in D68819, there was no record of it.

This patch introduces the capability of changing the arguments passed to
the script "on-the-fly" while processing a test file. In addition, an
"on/off" switch was added so that processing can be disabled for parts
of the file where the content is simply copied. The last extension is a
record of the invocation arguments in the auto generated NOTE. These
arguments are also picked up in a subsequent invocation, allowing
updates with special options enabled without user interaction.

To change the arguments the string UTC_ARGS: has to be present in a
line, followed by "additional command line arguments". That is
everything that follows UTC_ARGS: will be added to a growing list
of "command line arguments" which is reparsed after every update.

Event Timeline

jdoerfert created this revision.Oct 31 2019, 8:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 31 2019, 8:53 PM
Herald added a subscriber: bollu. · View Herald Transcript

This seems useful, thanks! Another case that would be nice is if there was a way to keep comments inside the autogenerated CHECK lines.

For example:

; CHECK-NEXT: ...
;; We generate a foo instruction and not a bar instruction here because...
; CHECK-NEXT: foo a, b, c
; CHECK-NEXT: ...

Right now if you re-run the update script the comment will be moved after all the CHECK lines. I wonder if there is a way to keep inline comments (maybe only if the CHECK: lines before and after didn't change?). Would this work with the UTC_ARGS: --turn off method?

llvm/utils/UpdateTestChecks/common.py
53

I think you need a r for the second string as well since otherwise newer python will complain that \s is not a valid escape sequence.

This seems useful, thanks! Another case that would be nice is if there was a way to keep comments inside the autogenerated CHECK lines.

For example:

; CHECK-NEXT: ...
;; We generate a foo instruction and not a bar instruction here because...
; CHECK-NEXT: foo a, b, c
; CHECK-NEXT: ...

Right now if you re-run the update script the comment will be moved after all the CHECK lines. I wonder if there is a way to keep inline comments (maybe only if the CHECK: lines before and after didn't change?). Would this work with the UTC_ARGS: --turn off method?

IMHO, that kind of comment belongs with the code that *generates* the output, rather than buried in the middle of a random test file. Separably, if it isn't clear what the test is testing, then perhaps some discussion at the top of the file/function seems appropriate, perhaps with links back to the relevant files/functions that are being tested. For example: "Please see the discussion inside of function foo in Bar.cpp for what is being tested below."

jdoerfert marked an inline comment as done.Nov 1 2019, 9:06 AM

This seems useful, thanks! Another case that would be nice is if there was a way to keep comments inside the autogenerated CHECK lines.

For example:

; CHECK-NEXT: ...
;; We generate a foo instruction and not a bar instruction here because...
; CHECK-NEXT: foo a, b, c
; CHECK-NEXT: ...

Right now if you re-run the update script the comment will be moved after all the CHECK lines. I wonder if there is a way to keep inline comments (maybe only if the CHECK: lines before and after didn't change?). Would this work with the UTC_ARGS: --turn off method?

It would not. The generation still happens in bulk. See below.

IMHO, that kind of comment belongs with the code that *generates* the output, rather than buried in the middle of a random test file. Separably, if it isn't clear what the test is testing, then perhaps some discussion at the top of the file/function seems appropriate, perhaps with links back to the relevant files/functions that are being tested. For example: "Please see the discussion inside of function foo in Bar.cpp for what is being tested below."

There are multiple use cases here for sure and the script addresses only one:
You generate check lines with the script, then you modify some of them for some reason. (Or you insert a test with manual check in lines into a file that has automatically generated ones.)
With this change you can "protect" the manual check lines in a reasonable way so we can rerun the script on files with the NOTE line.

llvm/utils/UpdateTestChecks/common.py
53

agreed

jdoerfert updated this revision to Diff 227488.Nov 1 2019, 11:03 AM

Lessons learned from actually using it (D68766)

Any comments (on this and the other [utils] patches)?

lebedev.ri requested changes to this revision.Fri, Dec 6, 6:05 AM

Now that i believe there's some testing infra for these utils, add/update tests for this?

This revision now requires changes to proceed.Fri, Dec 6, 6:05 AM