[PATCH] D116465: [SPIRV 6/6] Add one essential pass and the simplest tests

[Ilia Diachkov via Phabricator via llvm-commits]

iliya-diyachkov added a comment.

In D116465#3320976 <https://reviews.llvm.org/D116465#3320976>, @zuban32 wrote:

> In D116465#3320801 <https://reviews.llvm.org/D116465#3320801>, @iliya-diyachkov wrote:
>
>> @arsenm, when you suggested moving GlobalTypesAndRegNum' operations to AsmPrinter, did you implied that we need to insert SPIRVDummyGlobalPass after all SPIRVGISel passes to have whole MIR at the first run of AsmPrinter? Or is there another approach (or some pass configuration) to achieve the correct processing of the whole MIR in AsmPrinter?
>
> [I'm not Matt but] to me it seems like a hack which is not much better that the one we used to have with the MIR pass. I suppose the passes ordering would be the same if we just made the AsmPrinter a module pass instead of adding a dummy one, not sure about the potential consequences in the other targets though. @arsenm what do you think?

Thanks for your opinion, Aleksandr. I think this would be a smaller hack than we have now. I'll share the rough implementation in Khronos soon, so (since you know the full implementation of SPIRVGlobalTypesAndRegNumPass.cpp) you can appreciate the efficiency of this approach. In short, using it, we don't need to

1. create the dummy "global" function,
2. copy global MI from real functions to the "global" one,
3. make MIR consistent in the "global" and real functions after 1 and 2.

Also the implementation becomes shorter and looks simpler and more natural.

What about making AsmPrinter a module pass, it's a big target-independent change, I'm not sure that it can be justified only by the fact that SPIRV experimental backend can get some advantage.


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

https://reviews.llvm.org/D116465