[EXTERNAL][MLIR][ExecutionEngine] Guard obj.MLIRRocmExecutionEngineUtils references for static builds

[sudheerdevu]

Problem

A pure static configuration (BUILD_SHARED_LIBS=OFF) does not create the obj.* object-library targets. The ROCm execution-engine CMake, however, references obj.MLIRRocmExecutionEngineUtils unconditionally — for its private -Wno-* compile options and for its hip::host/hip::amdhip64 link libraries. Configuration then fails with an unknown-target error before the build starts.

Fix

Guard those references with if(TARGET obj.MLIRRocmExecutionEngineUtils) so the object-library target is only used when it is actually defined.

Impact

• Shared (default) builds: no functional change — the target always exists, so the guards are always true and this is a no-op.
• Static builds: configuration now succeeds, enabling the ROCm runner to be built with BUILD_SHARED_LIBS=OFF.
• Build-configuration only; no source or runtime code is touched.

Context

A static build is needed on ROCm 7.13 / gfx115x to avoid duplicate-LLVM symbol interposition between rocMLIR's vendored LLVM and ROCm's libLLVM/libamd_comgr at runtime. This guard is the only change required to make that configuration build; it is independent of, and harmless to, the default shared build.

[Copilot]

Pull request overview

This PR fixes CMake configuration failures in pure static builds (BUILD_SHARED_LIBS=OFF) by guarding references to the obj.MLIRRocmExecutionEngineUtils object-library target, which is not created in that configuration. This keeps the shared/default build behavior unchanged while allowing static configurations to configure successfully.

Changes:

• Guard target_compile_options(obj.MLIRRocmExecutionEngineUtils ...) behind if(TARGET obj.MLIRRocmExecutionEngineUtils).
• Guard target_link_libraries(obj.MLIRRocmExecutionEngineUtils ...) behind if(TARGET obj.MLIRRocmExecutionEngineUtils).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rocmlir-pr-reviewer]

Verdict: APPROVE -- submitted as COMMENT (automated reviews are advisory)  ·  Findings: 0 (0 Critical, 0 Major, 0 Minor)

---

Scope

Guards the four target_compile_options and the target_link_libraries references to obj.MLIRRocmExecutionEngineUtils in external/llvm-project/mlir/lib/ExecutionEngine/CMakeLists.txt behind if(TARGET obj.MLIRRocmExecutionEngineUtils). The obj.* object-library target only exists when libraries are built shared; a fully-static (BUILD_SHARED_LIBS=OFF) configure previously errored on the unknown target. Sole file changed.

Findings

No blocking issues found.

Notes

• The change is correct and idiomatic: if(TARGET ...) is the standard CMake guard for conditionally-defined targets, and the new outer guard cleanly wraps the existing inner CXX_SUPPORTS_* checks (lines 467-484) plus the link block (lines 494-498). For the default shared build the target always exists, so the guards are always true and the behavior is unchanged — matching the PR's stated no-op claim for shared builds.
• Minor / convention only (non-blocking): the checklist asks for external/ changes to be prefixed [EXTERNAL]. This PR is entirely under external/llvm-project/ (so it is already isolated from rocMLIR-proper changes, satisfying the "must be separate" intent), but the title is prefixed [MLIR][ExecutionEngine] rather than [EXTERNAL]. Worth aligning the title/commit prefix with the convention, but it does not affect correctness.
• This is a CMake/build-configuration change with no source or runtime impact, so the checklist's E2E/Lit/FileCheck test requirements do not apply.

CI status

No failing or cancelled checks in the pre-fetched status (only the in-progress auto-review detect check is present, which is expected).

[sudheerdevu]

Updated to address review feedback:

• Aligned the title and commit subject with the repository convention by adding the [EXTERNAL] prefix, since the change is entirely under external/llvm-project/. Force-pushed (single commit, same one-file diff).

On the CXX_SUPPORTS_CXX98_COMPAT_EXTRA_SEMI_FLAG note: that spelling is pre-existing in this file — the same name is already used in the untouched shared-build block (around line 424), where the value configured by check_cxx_compiler_flag is CXX_SUPPORTS_NO_CXX98_COMPAT_EXTRA_SEMI_FLAG. This PR only wraps the existing block in if(TARGET obj.MLIRRocmExecutionEngineUtils) and is intentionally a no-op for shared builds, so I left the spelling untouched to avoid changing behavior. Happy to correct it consistently across both blocks in a separate change if you'd prefer.

[sudheerdevu]

The ml-ci-internal.amd.com check reports "This commit cannot be built" on PR-2410/4, which looks like an infrastructure-level abort rather than a build problem with this change:

• The internal build ran for ~55 minutes (14:44 → 15:20 UTC, "being built"), then the top-level status went straight to error / "This commit cannot be built" at 15:39 UTC. The individual Linux stages (Build and Test, MIGraphX, Code coverage, …) never reported a final result — they are still showing "Building stage" — so no stage actually failed.
• Windows Component Build passed on this exact commit ("This commit looks good", 15:52 UTC), and the GitHub Actions detect check is green.
• The change itself is a no-op for shared builds (it only wraps obj.MLIRRocmExecutionEngineUtils references in if(TARGET ...) guards), and the develop merge did not modify this file.

@umangyadav could you please re-trigger the ml-ci-internal build when you have a moment? It looks like the agent dropped the job mid-run rather than a real failure. Thanks for merging develop in.

[umangyadav]

The ml-ci-internal.amd.com check reports "This commit cannot be built" on PR-2410/4, which looks like an infrastructure-level abort rather than a build problem with this change:

• The internal build ran for ~55 minutes (14:44 → 15:20 UTC, "being built"), then the top-level status went straight to error / "This commit cannot be built" at 15:39 UTC. The individual Linux stages (Build and Test, MIGraphX, Code coverage, …) never reported a final result — they are still showing "Building stage" — so no stage actually failed.
• Windows Component Build passed on this exact commit ("This commit looks good", 15:52 UTC), and the GitHub Actions detect check is green.
• The change itself is a no-op for shared builds (it only wraps obj.MLIRRocmExecutionEngineUtils references in if(TARGET ...) guards), and the develop merge did not modify this file.

@umangyadav could you please re-trigger the ml-ci-internal build when you have a moment? It looks like the agent dropped the job mid-run rather than a real failure. Thanks for merging develop in.

Triggered re-run

[sudheerdevu]

Merged current develop into this branch to clear the merge conflict.

The conflict was in external/llvm-project/mlir/lib/ExecutionEngine/CMakeLists.txt, where develop's AIROCMLIR-939 change (fixing the dead -Wno-c++98-compat-extra-semi suppression) overlapped this PR's obj.MLIRRocmExecutionEngineUtils TARGET guards.

The resolution keeps the TARGET guards from this PR and adopts develop's corrected CXX_SUPPORTS_NO_CXX98_COMPAT_EXTRA_SEMI_FLAG check name. The net diff against develop is unchanged — the same guard-only change (+22/-18 in that single file). The branch is MERGEABLE again, the existing approval still stands, and CI is re-running on the updated head.

[sudheerdevu]

This PR is green and ready: it is MERGEABLE, APPROVED, and the gating ml-ci-internal.amd.com check reports "This commit looks good" on the current head 99479ba (Windows Component Build and codecov also pass).

The seven Building stage entries (Build and Test, Tune MLIR kernels, Parameter sweeps, Benchmark and Report Performance, Archive weekly tuning perfDB, Code coverage, MIGraphX) are the usual non-gating tuning/benchmark placeholders — the same set was pending on the earlier develop-merge head, so they do not reflect a failure.

The merge conflict from the AIROCMLIR-939 change has been resolved and the branch is up to date with develop. Could a maintainer merge when convenient?

[sudheerdevu]

Hi @umangyadav — a gentle nudge on this one when you have a moment.

It is MERGEABLE and APPROVED, the earlier merge conflict is resolved, and the gating ml-ci-internal.amd.com check is green ("This commit looks good"), along with the Windows Component Build and codecov.

The 7 stages still showing pending are the usual non-gating tuning placeholders (Build and Test, Tune MLIR kernels, Parameter sweeps, Benchmark and Report Performance, Archive weekly tuning perfDB, Code coverage, MIGraphX) — the same set that never posts a terminal status. Could you merge (with override) when convenient? Thank you.

[umangyadav]

Hi @umangyadav — a gentle nudge on this one when you have a moment.

It is MERGEABLE and APPROVED, the earlier merge conflict is resolved, and the gating ml-ci-internal.amd.com check is green ("This commit looks good"), along with the Windows Component Build and codecov.

The 7 stages still showing pending are the usual non-gating tuning placeholders (Build and Test, Tune MLIR kernels, Parameter sweeps, Benchmark and Report Performance, Archive weekly tuning perfDB, Code coverage, MIGraphX) — the same set that never posts a terminal status. Could you merge (with override) when convenient? Thank you.

Hi @sudheerdevu would it be possible to create PR on llvm/llvm-project for this change as this is an external llvm related change ?

We will merge this PR but it would be good to have this change upstreamed as well.

[sudheerdevu]

Thanks @umangyadav, much appreciated on the merge.

On upstreaming: I looked into porting this to llvm/llvm-project, and this particular change doesn't have an upstream equivalent. It only touches the MLIRRocmExecutionEngineUtils / obj.MLIRRocmExecutionEngineUtils target, which is a rocMLIR-downstream addition and isn't present in upstream LLVM. Upstream mlir/lib/ExecutionEngine/CMakeLists.txt provides ROCm support via a different target (mlir_rocm_runtime, gated on MLIR_ENABLE_ROCM_RUNNER) and has no obj.MLIRRocmExecutionEngineUtils references to guard, so there's nothing to port for this specific fix.

If you instead meant contributing the MLIRRocmExecutionEngineUtils utilities themselves upstream, I'd be glad to help scope that as a separate change.

One note on CI: this change was fully green on PR-2410/7 (all stages, including ml-ci-internal.amd.com "looks good"). The red on PR-2410/8 appears to come from the develop CI-image migration (#2416, Ubuntu 24.04 / ROCm 7.2.4) pulled in with the latest develop merge — every Linux stage reports "Failed to build stage" while Windows passed, and this file is untouched by that merge. It should be safe to merge with override when convenient.