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

[Jaeho Kim]

================
@@ -48,19 +48,38 @@ struct GPUToSPIRVPass final : impl::ConvertGPUToSPIRVBase<GPUToSPIRVPass> {
   void runOnOperation() override;
 
 private:
+  spirv::TargetEnvAttr lookupTargetEnvInTargets(gpu::GPUModuleOp moduleOp);
+  spirv::TargetEnvAttr lookupTargetEnvOrDefault(gpu::GPUModuleOp moduleOp);
   bool mapMemorySpace;
 };
 
+spirv::TargetEnvAttr
----------------
oojahooo wrote:

Thank you for your comment.

Since I intentionally did not include `OrDefault` in the function name `GPUToSPIRVPass::lookupTargetEnvInTargets`, I wanted the function to only attempt to retrieve the target environment from the `"targets"` attribute, reflecting that limited scope directly in the name.

Then, I defined a separate `GPUToSPIRVPass::lookupTargetEnvOrDefault` function that calls it and falls back to `spirv::lookupTargetEnvOrDefault` if needed, making the fallback behavior more explicit.

However, if adding multiple new functions is considered undesirable, I'm happy to simplify the implementation by having only `GPUToSPIRVPass::lookupTargetEnvOrDefault` and letting it handle whole condition.

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