[Mlir-commits] [mlir] [mlir][SPIRV] Fix lookup logic `spirv.target_env` for `gpu.module` (PR #147262)

[Jaeho Kim]

oojahooo wrote:

> A rather naive question, as I'm not familiar with the GPU dialect. Is it possible for the target env to be present in both `"targets"` and as an op attribute at the same time? If so, does the look-up order matter? I’d hope the same target env is returned either way, but hypothetically if there is a different env in `"targets"` and in the op, I wonder which one should take a precedence?
> 
> Also, it'd be nice to have a test for the change. Probably in GPU to SPIR-V conversion test directory?

When the `gpu.module` op already has a target env as a dict-attr, running the `spirv-attach-target` pass can reproduce the situation you described. In such cases, I believe that the target env inserted by `spirv-attach-target` can be occurred without external intervention, so it should be given the highest priority during lookup. If that's the case, I think the flow in the committed code would be appropriate.

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