[PATCH] D122986: [UpdateTestChecks] Add NVPTX support in update_llc_test_checks.py

[Artem Belevich via Phabricator via llvm-commits]

tra added inline comments.


================
Comment at: llvm/utils/UpdateTestChecks/asm.py:184
+    # .visible .func (.param .align 16 .b8 func_retval0[32])
+    r'^(\.visible\s+)?\.func\s+(\([^\)]*\)\s*)?'
+
----------------
There may be other attributes (e.g. `.weak`) and they may come in different order.
It may be better to use something like `(\.(func|visible|weak|entry|noreturn|extern)\s+)`.


================
Comment at: llvm/utils/UpdateTestChecks/asm.py:422
+
 def get_run_handler(triple):
   target_handlers = {
----------------
It would be great to document what the function is expected to return.


================
Comment at: llvm/utils/UpdateTestChecks/asm.py:460
+      'csky': (scrub_asm_csky, ASM_FUNCTION_CSKY_RE, ':'),
+      'nvptx': (scrub_asm_nvptx, ASM_FUNCTION_NVPTX_RE, '(')
   }
----------------
The third field is a `label_suffix`, but it's not clear why it's `(` for NVPTX. 
AFAICT, PTX does use ':' for the labels: https://cuda.godbolt.org/z/34185xEbY

Looks like it's supposed to serve as a terminator for the `CHECK-LABEL` (i.e. it's a suffix in terms of FileCheck, but not necessarily in terms of the actual asm labels). 
This may be something to add to the description of the function return value.

I wonder if we can incorporate this suffix into a named regex match group, so we don't have to pass it explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122986