See bug 28234: https://bugs.llvm.org/show_bug.cgi?id=28234
Details
Diff Detail
Event Timeline
Why is a separate instruction needed for this? Do these also need to add the implicit m0 use?
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
---|---|---|
64–65 | This should go in the same header with the other of these InstrMapping function declarations |
I forgot about m0, thanks for pointing that out!
I thought about adding 'lds' modifier to existing instructions but decided against it for the following reasons:
- lds versions have different semantics and existing patterns are of no use.
- would need a flag in SIInstrFlags to identify MUBUF instructions that support 'lds'. This would be necessary for parser to check if instruction could use 'lds' and 'tfe'. Also note that there are not many free bits left in SIInstrFlags.
- would need to add or correct patterns for instructions that support lds to account for additional modifier.
- would need some validation code in parser to check that 'lds' is supported and consistent with 'tfe'.
Also it is trivial to specify implicit use of m0 for separate instructions. Not sure how to do that conditionally.
It is possible to significantly simplify proposed solution by adding 'lds' modifier to all buffer_load* instructions.
Do you suggest to go that way? Please advise.
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
---|---|---|
64–65 | I did that in the first implementation. Unfortunately the header - SIInstrInfo.h is not included in AsmParser. It has a lot of MachineInstr stuff that do not belong AsmParser. Should we include all that stuff for the sake of one declaration? |
We do stuff like Uses = !if(Foo, [EXEC], [VCC, EXEC]) other places
It is possible to significantly simplify proposed solution by adding 'lds' modifier to all buffer_load* instructions.
Do you suggest to go that way? Please advise.
I'm not sure. Separate instructions is probably better since it avoids needing to manually deal with the m0 constraint so is probably less error prone.
lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp | ||
---|---|---|
64–65 | We have some in AMDGPUBaseInfo.h (e.g. getNamedOperandIdx) |
Corrected issues found by Matt:
- added implicit use of M0 for lds opcodes;
- moved declaration of getMUBUFNoLdsInst to SIInstrInfo.h
Added:
buffer_load_sbyte v5, off, s[8:11], s3 lds tfe
// NOSICIVI: error: invalid operand for instruction
buffer_load_dword v5, off, s[8:11], s3 tfe lds
// NOSICIVI: error: invalid operand for instruction
This should go in the same header with the other of these InstrMapping function declarations