[AMDGPU][MC][GFX1013] Enable image_msaa_load

[dp]

Summary

Fixes this bug.

[rampitec]

LGTM, thanks!

[kosarev]

llvm/test/MC/AMDGPU/gfx1011_supported.s

1 ↗

(On Diff #434765)

Will it harm if we drop the _supported bit from the file name? +Same below.

[dp]

llvm/test/MC/AMDGPU/gfx1011_supported.s

1 ↗

(On Diff #434765)

These tests are a bit special and I added the suffix to distinguish them from regular tests. We also have tests with unsupported suffix which serve the same purpose. I'm planning to add supported tests for other GFX9 and GFX10 subtargets.

Another option I see is to move all supported tests to a separate folder (e.g. AMDGPU/subtargets), in this case the unsupported suffix will be unnecessary.

Any other ideas?

[kosarev]

llvm/test/MC/AMDGPU/gfx1011_supported.s

1 ↗

(On Diff #434765)

It seems so far we have been adding tests that make sure instructions are supported for particular targets without seeing them any special? Also, if this is a 'supported' kind of test, then what should we think are other positive encoding tests? Feels like this may be unnecessary complication.

[dp]

llvm/test/MC/AMDGPU/gfx1011_supported.s

1 ↗

(On Diff #434765)

You gave me a better idea. I think the supported tests may be further simplified by removing encoding checks. Thus they will only check that the specified instructions are supported, i.e. may be assembled without errors. This will also make these tests run faster. And after this change they will be special.

If nobody likes the idea, I'll rename the tests as you suggested.

[kosarev]

llvm/test/MC/AMDGPU/gfx1011_supported.s

1 ↗

(On Diff #434765)

If we want to make sure the support for the instructions is there, then why wouldn't we also want to check if they use the right encodings?

[dp]

llvm/test/MC/AMDGPU/gfx1011_supported.s

1 ↗

(On Diff #434765)

The question is where to stop. Instead of adding these supported tests we could have added RUN lines for missing subtargets to existing tests, but that would double or even triple test time according to my estimations. Note that we have 9 GFX10 subtargets with poor to none test coverage.

[kosarev]

llvm/test/MC/AMDGPU/gfx1011_supported.s

1 ↗

(On Diff #434765)

The point was, why where we have reasons to think the support for assembling instructions may be missed we shouldn't also assume the same applies to using the right encodings. And if it does apply, then the expected tests should be no special compared to what we already have.

To clarify, I don't suggest adding more RUN lines to those huge tests whose usefulness doesn't seem proportional to the amount of time they consume -- on top of that they make cases we are truly interested in less visible. The cases covered in the new tests do seem sufficient to me, and as long as we keep them this way, it should promise any problems in terms of testing times, shouldn't it?

[dp]

Removed irrelevant tests; updated GFX1013 specific tests.

[kosarev]

The tests look good to me, thanks.

llvm/lib/Target/AMDGPU/MIMGInstructions.td

 defm IMAGE_SAMPLE_CD_CL_O_G16   : MIMG_Sampler <mimgopc<MIMG.NOP, 0xed>, AMDGPUSample_cd_cl_o, 0, 1>;
 defm IMAGE_SAMPLE_C_CD_O_G16    : MIMG_Sampler <mimgopc<MIMG.NOP, 0xee>, AMDGPUSample_c_cd_o, 0, 1>;
 defm IMAGE_SAMPLE_C_CD_CL_O_G16 : MIMG_Sampler <mimgopc<MIMG.NOP, 0xef>, AMDGPUSample_c_cd_cl_o, 0, 1>;
 } // End OtherPredicates = [HasExtendedImageInsts,HasG16]
 
 //def IMAGE_RSRC256 : MIMG_NoPattern_RSRC256 <"image_rsrc256", mimgopc<0x7e>>;
 //def IMAGE_SAMPLER : MIMG_NoPattern_ <"image_sampler", mimgopc<0x7f>>;
 
-let SubtargetPredicate = isGFX10Only, OtherPredicates = [HasGFX10_BEncoding] in
+let SubtargetPredicate = isGFX10Only, OtherPredicates = [HasGFX10_AEncoding] in
 defm IMAGE_MSAA_LOAD_X : MIMG_NoSampler <mimgopc<MIMG.NOP, 0x80>, "image_msaa_load", 1, 0, 0, 1>;
 
 let OtherPredicates = [HasGFX10_AEncoding] in
 defm IMAGE_MSAA_LOAD : MIMG_MSAA_Load <mimgopc<0x18, MIMG.NOP>, "image_msaa_load">;
 
 let OtherPredicates = [HasGFX10_AEncoding] in {
 defm IMAGE_BVH_INTERSECT_RAY       : MIMG_IntersectRay<mimgopc<0x19, 0xe6>, "image_bvh_intersect_ray", 0, 0>;
 defm IMAGE_BVH_INTERSECT_RAY_a16   : MIMG_IntersectRay<mimgopc<0x19, 0xe6>, "image_bvh_intersect_ray", 0, 1>;

llvm/test/MC/AMDGPU/gfx1013.s

+// RUN: llvm-mc -arch=amdgcn -mcpu=gfx1013 -mattr=+wavefrontsize32,-wavefrontsize64 -show-encoding %s | FileCheck %s
+
+image_bvh64_intersect_ray v[5:8], v[1:16], s[8:11]
+// CHECK: [0x01,0x9f,0x9c,0xf1,0x01,0x05,0x02,0x00]
+
+image_bvh64_intersect_ray v[5:8], v[240:255], s[8:11] a16
+// CHECK: [0x01,0x9f,0x9c,0xf1,0xf0,0x05,0x02,0x40]
+
+image_bvh64_intersect_ray v[5:8], v[1:16], ttmp[12:15]
+// CHECK: [0x01,0x9f,0x9c,0xf1,0x01,0x05,0x1e,0x00]
+
+image_bvh64_intersect_ray v[39:42], [v50, v46, v23, v17, v16, v15, v21, v20, v19, v37, v40, v42], s[12:15]
+// CHECK: encoding: [0x07,0x9f,0x9c,0xf1,0x32,0x27,0x03,0x00,0x2e,0x17,0x11,0x10,0x0f,0x15,0x14,0x13,0x25,0x28,0x2a,0x00]
+
+image_bvh_intersect_ray v[252:255], v[1:16], s[8:11]
+// CHECK: [0x01,0x9f,0x98,0xf1,0x01,0xfc,0x02,0x00]
+
+image_bvh_intersect_ray v[5:8], v[248:255], s[8:11] a16
+// CHECK: [0x01,0x9f,0x98,0xf1,0xf8,0x05,0x02,0x40]
+
+image_bvh_intersect_ray v[5:8], v[1:16], ttmp[12:15]
+// CHECK: [0x01,0x9f,0x98,0xf1,0x01,0x05,0x1e,0x00]
+
+image_bvh_intersect_ray v[39:42], [v50, v46, v23, v17, v16, v15, v21, v20, v19, v37, v40], s[12:15]
+// CHECK: encoding: [0x07,0x9f,0x98,0xf1,0x32,0x27,0x03,0x00,0x2e,0x17,0x11,0x10,0x0f,0x15,0x14,0x13,0x25,0x28,0x00,0x00]
+
+image_msaa_load v[5:6], v[1:4], s[8:15] dmask:0x3 dim:SQ_RSRC_IMG_2D_MSAA_ARRAY
+// CHECK: [0x39,0x03,0x00,0xf0,0x01,0x05,0x02,0x00]
+
+image_msaa_load v[5:7], v[1:4], s[8:15] dmask:0x3 dim:SQ_RSRC_IMG_2D_MSAA_ARRAY tfe
+// CHECK: [0x39,0x03,0x01,0xf0,0x01,0x05,0x02,0x00]
+
+image_msaa_load v5, v[1:4], s[8:15] dmask:0x8 dim:SQ_RSRC_IMG_2D_MSAA_ARRAY d16
+// CHECK: [0x39,0x08,0x00,0xf0,0x01,0x05,0x02,0x80]
+
+image_msaa_load v[5:6], v[1:2], s[8:15] dmask:0x3 dim:SQ_RSRC_IMG_2D_MSAA_ARRAY a16
+// CHECK: [0x39,0x03,0x00,0xf0,0x01,0x05,0x02,0x40]
+
+image_msaa_load v[5:6], v[1:3], s[8:15] dmask:0x3 dim:SQ_RSRC_IMG_2D_MSAA
+// CHECK: [0x31,0x03,0x00,0xf0,0x01,0x05,0x02,0x00]
+
+image_msaa_load v[5:6], v[1:4], s[8:15] dmask:0x3 dim:SQ_RSRC_IMG_2D_MSAA_ARRAY unorm
+// CHECK: [0x39,0x13,0x00,0xf0,0x01,0x05,0x02,0x00]
+
+image_msaa_load v[5:6], v[1:4], s[8:15] dmask:0x3 dim:SQ_RSRC_IMG_2D_MSAA_ARRAY glc
+// CHECK: [0x39,0x23,0x00,0xf0,0x01,0x05,0x02,0x00]
+
+image_msaa_load v[5:6], v[1:4], s[8:15] dmask:0x3 dim:SQ_RSRC_IMG_2D_MSAA_ARRAY slc
+// CHECK: [0x39,0x03,0x00,0xf2,0x01,0x05,0x02,0x00]
+
+image_msaa_load v[5:6], v[1:4], s[8:15] dmask:0x3 dim:SQ_RSRC_IMG_2D_MSAA_ARRAY dlc
+// CHECK: [0xb9,0x03,0x00,0xf0,0x01,0x05,0x02,0x00]
+
+image_msaa_load v[5:6], v[1:4], s[8:15] dmask:0x3 dim:SQ_RSRC_IMG_2D_MSAA_ARRAY lwe
+// CHECK: [0x39,0x03,0x02,0xf0,0x01,0x05,0x02,0x00]