[llvm] [AsmPrinter] Do not emit label instructions after the function body if the target is SPIR-V (PR #107013)

Vyacheslav Levytskyy via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 16 02:31:52 PDT 2024


VyacheslavLevytskyy wrote:

> I am not familiar with SPIR-V but isn't this SPIR-V tooling issue? It's fairly common to have the following assembly construct
> 
> ```
> begin:
>   begin_instruction
> ...
>   end_instruction
> end:
> ```
> 
> then you can use `end-begin` to get the size of the function. Defining a label just past the last byte of the body is normal. I understand that each target may have its own special properties but this one might break LLVM's assumption badly.

The brief answer is that it's not a tooling issue for sure, but violation of the SPIR-V specification that is a way of hardcoded in the AsmPrinter code base.

A longer answer is that SPIR-V has one important property that makes it harder to develop the SPIR-V backend. It's a higher level intermediate representation, similar in its role to LLVM IR itself -- meaning that we expect it to be translated further, transformed to the specific target format to be used by a device. This property often leads to either lack of required support from available LLVM api (so that we have to keep our means of passing info between passes), or, the opposite, to the excess over-protecting kind of support where LLVM expects all the targets to behave similarly, and SPIR-V doesn't conform to the expectations.

In this specific case of the PR this means that we are to protect the resulting SPIR-V code from general assumptions to keep it conformant with requirements of the SPIR-V specification. We simply are not allowed to keep such kind of additional markup as `label:` in the output code. So, it's not a SPIR-V tooling issue, it's the case when LLVM general expectations/assumptions would lead to a violation of the resulting object format.

On another note, I'd gladly resolve this within the SPIR-V backend code reserve, but AsmPrinter literally hardcodes emission of labels in this case. Since I'm not sure that this deserves less local changes in the AsmPrinter code, I'd propose to insert this test.

https://github.com/llvm/llvm-project/pull/107013


More information about the llvm-commits mailing list