[PATCH] D129730: [SPIRV] add PrepareFunctions pass and update other passes
Aleksandr Bezzubikov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 21 13:08:26 PDT 2022
zuban32 added a comment.
In D129730#3669819 <https://reviews.llvm.org/D129730#3669819>, @iliya-diyachkov wrote:
> In D129730#3668881 <https://reviews.llvm.org/D129730#3668881>, @zuban32 wrote:
>
>> To me it seems as an unnecessarily huge change. Can't it be split, at least keep PrepareFunctions and other minor changes separate?
>
> Well, it looks big, but comparable to the patches from the initial series (and even smaller then some of them). By the way, the changes in most files are quite minor. Only 5 files are significantly changed (SPIRVAsmPrinter.cpp, SPIRVCallLowering.cpp, SPIRVGlobalRegistry.cpp, SPIRVInstructionSelector.cpp, SPIRVModuleAnalysis.cpp), and just one new pass is added.
>
> It's well tested and provides a good improvement in the stability and functionality of the SPIRV backend, so I wanted to introduce it before the 15th release. If the size is not a big issue and you don't mind, I will commit it as it is.
I won't insist this particular time, but its size just doesn't allow a proper review of the changes, so pls try to make changes better structured in the future. IMO the backend has enough code already to stop this 'Added feature X + many minor fixes' approach, we need to match the overall project's style of commits.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129730/new/
https://reviews.llvm.org/D129730
More information about the llvm-commits
mailing list