[PATCH] D144176: [AMDGPU] Add cross-project-tests for WMMA builtins

Scott Linder via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 16 12:28:33 PST 2023


scott.linder added a comment.

This is great, I also didn't realize there were such a thing :)

A few nits, but otherwise LGTM



================
Comment at: cross-project-tests/CMakeLists.txt:97
 set_target_properties(check-intrinsic-headers PROPERTIES FOLDER "Tests")
+if ("AMDGPU" IN_LIST LLVM_TARGETS_TO_BUILD)
+  add_lit_testsuite(check-amdgpu-cross "Running AMDGPU cross-project-tests"
----------------
The approach in LLVM seems to be to define all `check-*` targets unconditionally, and let lit sort out what it supported. I think we can drop the condition here (and the result will be that 0 tests are run when building `check-amdgpu-cross` without the AMDGPU target).

For example:
```
$ cmake -S llvm -B build -DLLVM_TARGETS_TO_BUILD=X86
$ cmake --build build check-llvm-codegen-amdgpu
Testing Time: 2.08s
Unsupported: 3075
```


================
Comment at: cross-project-tests/CMakeLists.txt:102
+    )
+  set_target_properties(check-amdgpu-cross PROPERTIES FOLDER "Tests")
+endif()
----------------
I'd prefer `check-cross-amdgpu` so we match the directory hierarchy, as that seems to be the convention elsewhere (i.e. it is `check-llvm-codegen-amdgpu`, rather than `check-amdgpu-llvm-codegen`).

The debuginfo/intrinsic-headers targets appear to break this convention but they are just aliases for convenience.


================
Comment at: cross-project-tests/amdgpu/builtins-amdgcn-wmma-w32.cl:1
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx1100 -target-feature +wavefrontsize32 -DWMMA_GFX1100_TESTS -S -o - %s | FileCheck %s --check-prefix=CHECK-GFX1100
----------------
These REQUIRES can be factored out to the lit.cfg for the amdgpu directory


================
Comment at: cross-project-tests/amdgpu/lit.local.cfg:1
+if 'clang' not in config.available_features:
+    config.unsupported = True
----------------
i.e. here add ` or not 'AMDGPU' in config.roots.targets`


================
Comment at: cross-project-tests/lit.cfg.py:25
 # suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.c', '.cpp', '.m']
+config.suffixes = ['.c', ".cl", '.cpp', '.m']
 
----------------
Nit: use single quotes to match other literals


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144176



More information about the llvm-commits mailing list