[PATCH] D148216: Add support for annotations in UpdateTestChecks (NFC)

[Dan Liew via Phabricator via llvm-commits]

delcypher added inline comments.


================
Comment at: clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected:12
+// CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr [[X]], align 4
+// CHECK-NEXT:    ret i32 [[TMP1]]
+//
----------------
@hnrklssn I just noticed we don't have a `CHECK` for what `META2` actually refers to. Should we?

Not something that has to be fixed in this patch, more just an observation.


================
Comment at: llvm/utils/UpdateTestChecks/common.py:703
   def get_ir_prefix_from_ir_value_match(self, match):
-    return self.ir_prefix, self.check_prefix
+    return re.search(self.ir_prefix, match[0])[0], self.check_prefix
 
----------------
Is this call to `re.search(...)` guaranteed to always succeed?


================
Comment at: llvm/utils/UpdateTestChecks/common.py:779
     NamelessValue(r'ACC_GRP'    , '!' , r'!llvm.access.group ' , r'![0-9]+'             , None                 ) ,
+    NamelessValue(r'META'       , '!' , r'![a-z.]+ '           , r'![0-9]+'             , None                 ) ,
 ]
----------------
There may be some value in having `!annotations` matched explicitly as well as there being a fallback. In the current patch it looks like:

* `metadata` is assigned variables with the `META` prefix on `CHECK` lines
* `!annotation` is assigned variables with the `META` prefix on `CHECK` lines
* Anything else that matches `r'![a-z.]+ ' `  is assigned variables with the `META` prefix on `CHECK` lines

When looking a large test having everything lumped into `META` variables could make the generated tests a little harder to read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148216