Details
Diff Detail
Event Timeline
| lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
|---|---|---|
| 1581 | Typo: subtaget | |
| 1587–1589 | I think you need to be careful here if the immediate is == 255. It is still is uint<8>, but the encoding value will end up reading the next 4 bytes of instruction instead of the right value. | |
| test/CodeGen/AMDGPU/smrd.ll | ||
| 19 | These should probably be split into SI and CI lines. I think on SI this will end up using a 32-bit encoding and will have to use on a 64-bit one on CI for 255 | |
| test/MC/AMDGPU/smrd.s | ||
| 11 | Should probably include a test with 0xff too | |
| lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
|---|---|---|
| 1587–1589 | Are you concerned that the value 0xff will be read as SRC_LITERAL rather than the actual offset? Because there is no way to specify SRC_LITERAL using the assembler. | |
| test/CodeGen/AMDGPU/smrd.ll | ||
| 19 | I double -checked and 32-bit immediates are only supported on CI. I updated this test to make sure it is using the 32-bit encoding of SMRD. | |
| lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
|---|---|---|
| 1587–1589 | Yes. My understanding was you can't use 255 because that is the same as SRC_LITERAL and both are set in the same immediate field. The meaning of 255 would then change meaning depending on SI/CI. | |
| test/CodeGen/AMDGPU/smrd.ll | ||
| 19 | Because 255 is SQ_LITERAL on CI I think it will be 64-bit there | |
| lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
|---|---|---|
| 1587–1589 | The meaning of 255 is controled by the IMM bit, so there shouldn't be an issue. 8-bit offset (0xff) 32-bit offset | |
| lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
|---|---|---|
| 1587–1589 | OK, I didn't notice that bit. I think the assembler should probably just be smart about it and turn it on only if it sees an immediate that won't fit in 8-bits | |
| lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
|---|---|---|
| 1587–1589 | The assembler already does this. | |
Typo: subtaget