Added support for 21-bit signed SMEM offsets (GFX9+).
Details
Diff Detail
Event Timeline
llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp | ||
---|---|---|
1313–1315 | This code should probably be corrected as well but I do not know if codegen is ready for this change. |
llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp | ||
---|---|---|
369 ↗ | (On Diff #261878) | My intention was to state explicitly that an immediate is expected. But this is a matter of taste. I’ll remove the assert. |
372–373 ↗ | (On Diff #261878) | This is not strictly necessary. However AMD documentation states that unused bits in instruction encoding should be zero. An MC instruction may come directly from codegen, and I’m not sure it does SMEM offset validation. Should I replace this truncation with an assert? |
llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp | ||
---|---|---|
372–373 ↗ | (On Diff #261878) | I would assume codegen would catch this in a verifier, and the assembler verifier would prevent this from getting here, so this should probably be an assert |
Restored getSMEMOffsetEncoding; replaced offset truncation with an assert as suggested by Matt.
LGTM with assert nit
llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp | ||
---|---|---|
370 ↗ | (On Diff #262114) | isUINt<20> would be clearer |
I think we're missing the same checks in the MachineVerifier. Can you avoid spreading the hardcoded number of bits to multiple places?