[PATCH] D125992: [AMDGPU] gfx11 FLAT Instructions

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/test/MC/AMDGPU/flat-gfx11.s:5
+// RUN: not llvm-mc -arch=amdgcn -mcpu=gfx1030 2>&1 %s | FileCheck --check-prefix=VI-GFX9_10-ERR --implicit-check-not=error: %s
+// RUN: not llvm-mc -arch=amdgcn -mcpu=gfx1100 -show-encoding %s | FileCheck --check-prefix=GFX11 %s
+
----------------
Joe_Nash wrote:
> rampitec wrote:
> > Ditto 'not'
> This test contains positive tests for gfx11, negative tests for gfx11, and negative tests for previous architectures. I it enables detection of errors with all of those. Is that ok to land as is?
It has positive tests, but all the run lines prepended with 'not'. This is not right. I believe GFX11 run line shall not have it.


================
Comment at: llvm/test/MC/AMDGPU/gfx11_flat.s:1
+// RUN: llvm-mc -arch=amdgcn -mcpu=gfx1100 -mattr=+wavefrontsize32,-wavefrontsize64 -show-encoding %s | FileCheck --check-prefix=GFX11 %s
+// RUN: llvm-mc -arch=amdgcn -mcpu=gfx1100 -mattr=-wavefrontsize32,+wavefrontsize64 -show-encoding %s | FileCheck --check-prefix=GFX11 %s
----------------
Joe_Nash wrote:
> rampitec wrote:
> > There seems to be too much tests doing the same, isn't it?
> I have looked at the test cases in all these added test files, and they are all unique. 
Test names are misleading. I cannot immediately tell what's the difference between flat-gfx11.s and gfx11_flat.s


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125992