[PATCH] D152805: [AMDGPU][GFX11] Add test coverage for 16-bit conversions, part 5.
Joe Nash via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 13 14:06:48 PDT 2023
Joe_Nash added inline comments.
================
Comment at: llvm/test/CodeGen/AMDGPU/function-returns.ll:1
-; RUN: llc -march=amdgcn -mtriple=amdgcn-- -mcpu=hawaii -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,CI %s
-; RUN: llc -march=amdgcn -mtriple=amdgcn-- -mcpu=fiji -mattr=-flat-for-global -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX89 %s
-; RUN: llc -march=amdgcn -mtriple=amdgcn-- -mcpu=gfx900 -mattr=-flat-for-global -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX89,GFX9 %s
-
-; GCN-LABEL: {{^}}i1_func_void:
-; GCN: buffer_load_ubyte v0, off
-; GCN-NEXT: s_waitcnt
-; GCN-NEXT: s_setpc_b64
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
+; RUN: llc -march=amdgcn -mtriple=amdgcn-- -mcpu=hawaii -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX789,CI %s
----------------
kosarev wrote:
> arsenm wrote:
> > Switching to generated checks should be done in a precommit
> I don't see how that can be useful in this case. There are no functional changes and the new checks are all as visible here as they would be in a separate patch.
I think adding a GFX11 runline to existing tests (manual or autogenerated) is fine as long as the test is relevant to GFX11. The questionable part for me comes in when we are converting manually written tests to autogenerated. These need some level of judgement to determine if the clarity of the test is lost by making it auto generated. I think it would ease review if you resorted tests in the unlanded patches in this series into a format where a patch contains only one of 1) adding GFX11 runlines or 2) converting a test to autogenerated and adding GFX11 runlines. Patches with 1) only should be easy to approve. Then we can take a closer look at if those cases in 2) should really be manually updated.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152805/new/
https://reviews.llvm.org/D152805
More information about the llvm-commits
mailing list