The second patch in the effort of upstreaming the SPIR-V backend to the trunk. The patch adds initial TableGen sources: SPIRVInstrFormats.td, SPIRVInstrInfo.td, SPIRVRegisterBanks.td, SPIRVRegisterInfo.td, SPIRVEnums.td, and the main SPIRV.td. Please note that the patch will need to be amended/merged after D115009: [SPIRV 1/6] Add stub for SPIRV backend gets accepted and committed.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Can't see anything wrong with it but I don't know the SPIRV ISA, so...
llvm/lib/Target/SPIRV/SPIRVInstrInfo.td | ||
---|---|---|
68 | I'm guessing genV also defines SF?Cond and SI?Cond? Ie. the VS and VI are in addition to those. |
llvm/lib/Target/SPIRV/SPIRV.td | ||
---|---|---|
44 | Several files in this patch are missing their newline at EOF. | |
llvm/lib/Target/SPIRV/SPIRVInstrInfo.td | ||
89 | The numbering doesn't correspond to the latest unified spec [1]. It is probably impractical to keep those numbers in sync as the spec keeps evolving. But perhaps we can provide a spec version number to avoid confusion? [1] https://www.khronos.org/registry/SPIR-V/specs/unified1/SPIRV.html#_miscellaneous_instructions | |
152 | Remove 1 newline here. | |
241 | This can probably be removed? | |
493–495 | This can probably be removed? | |
595 | Please indent. | |
692–693 | This patch doesn't add any instructions for these, so we could just as well remove these comments? Or alternatively, if we plan to support them, mark as TODO. | |
743 | Not sure if this comment is relevant? |
llvm/lib/Target/SPIRV/SPIRVInstrInfo.td | ||
---|---|---|
68 | Not sure I understand you comment. This multiclass may be used to generated any of scalar/vector integer/float versions of ternary instructions with either scalar or vectors first operand. Now as I'm looking into it it seems quite redundant and irrelevant. It was introduced when we planned to move all of the selection stuff to tblgen from plain C++, but now it seems unrealistic so we may select OpSelect with C++ and remove this whole mclass. |
llvm/lib/Target/SPIRV/SPIRVInstrInfo.td | ||
---|---|---|
68 | What I meant is that, in this definition, genV would have all 8 SF?Conv, SI?Conv, VS?Conv and VI?Conv, while not-genV would only have the initial 4. I just wanted to make sure that was the intent, rather than genV only having the latter 4. |
All comments seem to have been addressed and the patch itself is very mechanical in nature, so LGTM.
Several files in this patch are missing their newline at EOF.