This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][GlobalISel] Fix selection of image sample g16 instructions
ClosedPublic

Authored by mbrkusanin on Feb 9 2023, 7:04 AM.

Details

Reviewers
foad
arsenm
Summary

Pre-GFX10 A16 modifier would imply G16. From GFX10 and onward there are
separate instructions for 16bit gradients. This fixes the condition for
selecting G16 opcodes. Also stop adding G16 flag to instructions that do not
use gradients for GFX10 onward.

Diff Detail

Event Timeline

mbrkusanin created this revision.Feb 9 2023, 7:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2023, 7:04 AM
mbrkusanin requested review of this revision.Feb 9 2023, 7:04 AM
arsenm accepted this revision.Feb 9 2023, 7:19 AM
This revision is now accepted and ready to land.Feb 9 2023, 7:19 AM
foad added inline comments.Feb 9 2023, 8:18 AM
llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
4905

This simplifies to GradTy == S16 && (!ST.hasG16() || BaseOpcode->Gradients) - but perhaps your way is more readable.

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.image.sample.g16.a16.dim.ll
860

How would these new tests fail without your patch? Would they fail machine verification?

mbrkusanin added inline comments.Feb 9 2023, 8:22 AM
llvm/test/CodeGen/AMDGPU/llvm.amdgcn.image.sample.g16.a16.dim.ll
860

GlobalISel was selecting non g16 variant without a crash or fail. There were no test cases with a16 and g16.