Page MenuHomePhabricator

[AMDGPU][GISel] Add Identity BUILD_VECTOR Combines
ClosedPublic

Authored by Pierre-vh on Sep 30 2022, 5:27 AM.

Details

Summary

Folds-away BUILD_VECTOR-related noops in the post-legalizer combiner.

Depends on D134433

Diff Detail

Event Timeline

Pierre-vh created this revision.Sep 30 2022, 5:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2022, 5:27 AM
Pierre-vh requested review of this revision.Sep 30 2022, 5:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2022, 5:27 AM
arsenm added inline comments.Sep 30 2022, 5:30 AM
llvm/lib/Target/AMDGPU/AMDGPUCombine.td
48–52

Can move this to the generic combiner

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp
333

Shouldn't need to bother with getDefIgnoringCopies, copy of copy is folded out during combining. We should have an m_Undef or m_ImplicitDef too if there isn't one already

llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.image.load.1d.d16.ll
947–949

Should remove this and remove the unused prefixes

llvm/test/CodeGen/AMDGPU/GlobalISel/postlegalizer-combiner-buildvector-identities.mir
2

Don't need -global-isel with -run-pass

llvm/test/CodeGen/AMDGPU/GlobalISel/postlegalizer-combiner-trunc-bitcast-buildvector.mir
2

Don't need -global-isel with -run-pass

Pierre-vh updated this revision to Diff 464237.Sep 30 2022, 5:52 AM
Pierre-vh marked 5 inline comments as done.

Comments

arsenm added inline comments.Sep 30 2022, 6:00 AM
llvm/lib/Target/AMDGPU/AMDGPUCombine.td
48

Left behind AMDGPU copy

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp
308–309

Left behind the AMDGPU copy

LGTM except for the leftover copy

Pierre-vh updated this revision to Diff 464240.Sep 30 2022, 6:18 AM
Pierre-vh marked an inline comment as done.

Comment

llvm/lib/Target/AMDGPU/AMDGPUCombine.td
48

That's another combine for (G_TRUNC (G_BITCAST (G_BUILD_VECTOR x, y), unless that one can also go in the generic combiner?

arsenm added inline comments.Sep 30 2022, 6:29 AM
llvm/lib/Target/AMDGPU/AMDGPUCombine.td
48

Yes, all of this can. It's not doing anything that target specific and uses no custom opcodes

Pierre-vh updated this revision to Diff 464252.Sep 30 2022, 6:44 AM
Pierre-vh marked 2 inline comments as done.

Move all to Helper

arsenm accepted this revision.Sep 30 2022, 6:53 AM
This revision is now accepted and ready to land.Sep 30 2022, 6:53 AM
This revision was landed with ongoing or failed builds.Sep 30 2022, 7:07 AM
This revision was automatically updated to reflect the committed changes.