This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/GlobalISel: Fix constant bus restriction errors for med3
ClosedPublic

Authored by Petar.Avramovic on Nov 29 2021, 5:23 AM.

Details

Summary

Detected on targets older then gfx10 (e.g. gfx9) for constants that are
too large to be inlined (constant are sgpr by default).
In med3 combine it is expected that regbankselect maps all operands of
min/max we try to match to vgpr. However constants are mapped to sgpr
and there will be a sgpr-to-vgpr copy. Matchers look through sgpr-to-vgpr
copies and return sgpr and these break constant bus restriction.
Build med3 with all vgpr operands. Use existing sgpr-to-vgpr copies for
matched sgprs. If there is no such copy (not expected) build one.

Diff Detail

Event Timeline

Petar.Avramovic requested review of this revision.Nov 29 2021, 5:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 29 2021, 5:23 AM
arsenm added inline comments.Nov 29 2021, 11:21 AM
llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp
155–157

The subtarget should probably be cached in the combiner helper

156

My intention was globalisel should not need to think about the constant bus restriction at all because it will be quickly become unmanageable. All code would need to be exactly aware of how the instructions will be selected, which is impossible.

All VALU instructions should use all VGPR (or VCC) mappings

167

Should not consider individual mappings, everything just needs to be VGPR

168–171

Probably should have a constrain or copy to VGPR helper

Petar.Avramovic edited the summary of this revision. (Show Details)

Make all operands vgpr for med3. Find already existing copies to vgpr for constants.

foad added inline comments.Nov 30 2021, 3:52 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/regbankcombiner-smed3.mir
16–26

What is the difference between GFX9 and GFX10 here? Can they use a common prefix?

Use same prefix for gfx9 and gfx10 in mir test.

arsenm accepted this revision.Dec 1 2021, 5:52 AM
arsenm added inline comments.
llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp
77–82

This feels like more papering over unhandled constant regbankselect

This revision is now accepted and ready to land.Dec 1 2021, 5:52 AM