[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