In early May @nikic committed a change to regenerate the checks in the InstCombine tests. Today, I came across an error that was introduced by that change. This looks like a short-coming in the script that generates the checks.
The test with the problem is llvm/test/Transforms/InstCombine/pow-exp.ll. Before the change, this test contained the following lines:
; Do not change 0xBFE0776{{.*}} to the exact constant, see PR42740
; CHECK-NEXT: [[MUL:%.*]] = fmul nnan ninf afn double [[E:%.*]], 0xBFE0776{{.*}}
The update script kept the comment, but made the change that the comment said not to make. Nikita’s change updated a huge amount of tests, so I don’t think it would have been reasonable to expect anyone to notice this while reviewing the changes.
I don’t know if there are any other cases like this. I would expect so. In this case, the test is constant folding a call to pow(). The problem is that the implementation of this function used by the compiler doesn’t necessarily return correctly rounded results, so checking for the exact result found when updating the test can lead to failures when the test is compiled with a different math runtime library.
Can we change update-test-checks.py to have it preserve wildcards in the constant checks?
I think this is user error; the point of auto-generated checks is that they’re auto-generated. If you’re hand-editing the checks, you should remove the “autogenerated by update-test-checks.py” line at the top of the file. And maybe add a note explaining why, if the checks were originally generated.
Maybe it makes sense to make update-test-checks.py try find CHECK lines that don’t appear to be automatically generated, and refuse to update them unless the user explicitly requests it.
Now I’m curious: Is there a way to prevent UTC from auto-generating checks for a file? because in a case like this where something is a little flaky, you wouldn’t want someone to come along and ruin your carefully hand-written checks.
There’s an --update-only flag that tells UTC to only updates tests which are auto-generated.
If you write something like ; NOTE: Assertions have been autogenerated by hand, UTC will refuse to update it. This is meant to prevent accidentally switching from update_llc_test_checks to update_test_checks, or something like that, but you can also use it like this. Maybe we could add better syntax for it.