[PATCH] D116462: [SPIRV 3/6] Add MC layer, object file support and InstPrinter

[Ilia Diachkov via Phabricator via llvm-commits]

iliya-diyachkov added a comment.

In D116462#3376700 <https://reviews.llvm.org/D116462#3376700>, @MaskRay wrote:

> If you use a recent Clang to build llvm-project, there are several warnings:
>
>   llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4859:11: warning: enumeration value 'SPIRV' not handled in switch [-Wswitch]
>   llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2106:11: warning: enumeration value 'SPIRV' not handled in switch [-Wswitch]
>   llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:6368:11: warning: enumeration value 'IsSPIRV' not handled in switch [-Wswitch]
>   
>   # I did not check whether this was due to a previous patch
>   llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVBaseInfo.cpp:532:3: warning: default label in switch which covers all enumeration values [-Wcovered-switch-default]

Thanks, I'll fix.

> Does SPIRV need TargetLoweringObjectFileSPIRV?

I think not, since SPIRV does not use sections (other than text), relocations and other related things.

> When GOFF was added in D106380 <https://reviews.llvm.org/D106380>, there was a test in test/CodeGen/SystemZ
> This patch introduces a good chunk of MC stuff but there seems no accompanying test.

I think we cannot add a similar test in this patch for two reasons:

- GOFF was added to existing targets with implemented set of target passes. SPIRV target (in this 3rd patch) has not the passes yet and cannot generate asm and obj files.
- All content of SPIR-V modules in the form of text files are instructions. Thus, any rational test should contain SPIR-V instructions. AsmPrinter is implemented in the 4th patch, so theoretically we could add a simple test in the 4th patch (but it would require moving some code from the 5th/6th to the 4th patch of the series).

However, these MC changes are covered by the tests in the 6th patch, and all 6 patches will be committed to LLVM repository simultaneously, so I'm not sure what benefits do we really get by moving some tests from the 6th patch to the earlier ones.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116462/new/

https://reviews.llvm.org/D116462