This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][GlobalISel] Update applyMappingImpl for G_ABS and type v2s16
ClosedPublic

Authored by Acim-Maravic on Jul 20 2023, 10:23 AM.

Details

Summary

For G_ABS with type v2s16 and sgpr inputs break down into two s32 G_ABS
instructions.

Diff Detail

Event Timeline

Acim-Maravic created this revision.Jul 20 2023, 10:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2023, 10:23 AM
Acim-Maravic requested review of this revision.Jul 20 2023, 10:23 AM
arsenm added inline comments.Jul 20 2023, 11:08 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.abs.vector.ll
292–293 ↗(On Diff #542581)

Why is it legal if there's no vector abs? RegBankSelect doesn't need to consider always illegal operations

llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.abs.vector.ll
292–293 ↗(On Diff #542581)

Lowering is different
sgpr: unpack v2i16, 2 x abs_i32, pack to v2i16
vgpr: use v_pk sub and max

lgtm with some nits. Can you fix the patch title to something that isn't "fix"? I don't think anything was wrong here, just sub-optimal

llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
2427–2435

indentation looks off

llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.abs.vector.ll
5–8 ↗(On Diff #543489)

Should just add these cases to the existing llvm.abs.ll tests

foad added a comment.Jul 27 2023, 3:05 AM

Please upload patches with full context - either git diff -U9999999 or use arc diff.

Acim-Maravic retitled this revision from [AMDGPU][GlobalISel] Fix applyMappingImpl function for G_ABS and type v2s16 to [AMDGPU][GlobalISel] Update applyMappingImpl for G_ABS and type v2s16.
Acim-Maravic edited the summary of this revision. (Show Details)
arsenm accepted this revision.Jul 27 2023, 9:48 AM
This revision is now accepted and ready to land.Jul 27 2023, 9:48 AM
This revision was landed with ongoing or failed builds.Aug 2 2023, 3:31 AM
This revision was automatically updated to reflect the committed changes.