Thanks @rengolin! I guess in this case we have to see what @steleman and @joelkevinjones think and if they are interested (As well as other reviewers).

Hi all!

Added a clearer explanation of how execution mode is
converted when there are no values associated with it.

Fixed a different global naming when the parent module does not have a name.

After some more thinking and further reading, I realise that entry point information is not really needed. However, it is important to keep execution mode info. This changes the conversion pattern as follows:

• spv.EntryPoint is simply removed
• Info from spv.ExecutionMode is used to create a global variable, which now looks like:

struct {
   int32_t executionMode;
   int32_t values[];          // optional values
};

Execution Mode ops may contain important information about the entry point, which we may want to preserve. An example can be LocalSize which indicates the work-group size. This can be reused when handling multi-threaded code conversion.

@mravishankar So this patch should be good to land then?

Changed includes to match new file locations in lower-host-to-llvm pass.

Updated struct types according to https://reviews.llvm.org/D87206.

In D86112#2331714, @mravishankar wrote:

Coming around to landing this change now. This was causing some issues with builds (internal builds within Google) and I figured a better placement of the files works better. See this patch : https://reviews.llvm.org/D89448 for the changes. I will commit that change instead. The dependent change should be modified to suit this though. I can take care of that as well

Moved files around as suggested by @mravishankar.

In D88544#2309019, @dcaballe wrote:

Hey! Thanks for the patch! I wondered if it would make sense to move this utility to a public utility header. I found myself needing it locally and I wouldn't like to just copy-paste it. There is another topological sort for ops under 'include/mlir/Analysis/SliceAnalysis.h'. Maybe we could move them both to 'include/mlir/IR/Utils.h'? WDYT?

Splitted the pattern into two and removed unnecessary helper functions.

Rebased and addressed comments.

Addressed comments and reused llvm::ReversePostOrderTraversal for topological sorting.

Thanks!

Addressed comments:

In D86108#2242754, @mravishankar wrote:

Latest patch works, but I still need the fixes to the builds (actually the fixes to GPUTransforms/CMakeLists.txt above is for the patch earlier in the stack).

Thanks George for making this available!

In D86108#2241210, @mravishankar wrote:

Trying this change out, I needed the following changes to get this to build locally. Can you verify this is needed?

index 560dcae8200..41f5c8a9ce5 100644
--- a/mlir/lib/Conversion/GPUCommon/CMakeLists.txt
+++ b/mlir/lib/Conversion/GPUCommon/CMakeLists.txt
@@ -34,6 +34,8 @@ add_mlir_conversion_library(MLIRGPUToGPURuntimeTransforms
   MLIRIR
   MLIRLLVMIR
   MLIRPass
+  MLIRSPIRV
   MLIRSupport
   MLIRStandardToLLVM
+  MLIRSPIRVToLLVM
 )
diff --git a/mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt b/mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
index 69080ae66dc..8e0c2a6ad67 100644
--- a/mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
+++ b/mlir/tools/mlir-spirv-cpu-runner/CMakeLists.txt
@@ -15,6 +15,7 @@ if (MLIR_SPIRV_CPU_RUNNER_ENABLED)
   get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
 
   target_link_libraries(mlir-spirv-cpu-runner PRIVATE
+    LLVMLinker
     ${conversion_libs}
     ${dialect_libs}
     MLIRAnalysis

I am building locally without these changes and it is working for me?

Rebased on the correct version of LowerHostToLLVMPass.

In D86108#2241210, @mravishankar wrote:

Trying this change out, I needed the following changes to get this to build locally. Can you verify this is needed?

I will have a look at this one as well!

Added a missing change.

Fixed StringRef error.

In D86108#2241210, @mravishankar wrote:

Any ideas about what I might be missing here?

Rebase against master.

Addressed comments and rebased on master to pick up committed patches.

In D86108#2240916, @mravishankar wrote:

Looking at the stack here, I think only the change below this one in the stack (which is good to go) and this would probably need me to submit this. It would be good to commit the other two dependent changes and then I could try these two out locally.

Yes, only the pass converting gpu.launch_func and host code is needed. I am committing all accepted patches now :)

Fixed clang-tidy error.

Addressed comments.

Updated the runner based on the previous patches and added a test for 3D memrefs with floats.

Rebase on top of other patches and minor changes: variables/function names now keep __spv__ from SPIR-V module.

Fixed the clang-tidy error.

Rebased on top of the SPIR-V module name patch to include the symbol name of the module into the new name of the global variable.

Addressed comments.

Removed EncodeDescriptorSetsPass.

Added missing file.

This is a big update to the pass structure. Now it is run after GPU to SPIR-V
conversion (and all ABI lowering, etc.). This allows to use SPIR-V module,
binds of global variables for kernel call emulation. Also, we can have multiple
kernels in the program (To fully run these the linking has to be updated).

Addressed comments.