[PATCH] D69687: [NFCI] Rerun update_test_checks.py on all Analysis tests

Thu Oct 31 15:58:24 PDT 2019

jdoerfert marked an inline comment as done.
jdoerfert added inline comments.

================
Comment at: llvm/test/Analysis/ConstantFolding/math-1.ll:16
+; CHECK-LABEL: define {{[^@]+}}@f_asinf(
+; CHECK-NEXT:    ret float 0x3FF921FB60000000
 ;
----------------
nikic wrote:
> I guess a problem with doing these kinds of mass-regenerations is that it's easy to miss meaningful diffs that hide in between. Like this one -- I'm not sure why (presumably some host dependent float precision issue), but the use of `{{.+}}` here probably needs to be preserved.
I think, at least for this diff, it is not hard to spot/identify any change that does not include `define {{[^@]+}}`. While that might not be always true, we can probably write decent filters for changes of this kind.

Wrt. this actual change, I see your point. Though, I think that it is more of a problem with our usage of this script (or, arguably, my intended usage).

On the one hand, the files "says" that check lines are auto generated but in reality they are not. If we cannot regenerate them, I'd say the NOTE is relatively useless. Maybe we don't need to rerun on this file ever again, then there is no need to mark  the file with the NOTE. If the idea (of the note) is to tell people to run the update script when they change something, then it is unclear why we have manual changes like these that do not survive a rerun.
Maybe we shouldn't have such manual changes, so you see the actual diff. Maybe we should have manual changes that survive the update script.
Or maybe I misunderstand what the script and the NOTE is supposed to do.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69687