[AIROCMLIR-939] Fix dead -Wno-c++98-compat-extra-semi suppression

[bogdan-petkovic]

Motivation

A variable-name typo made the -Wno-c++98-compat-extra-semi suppression dead code. check_cxx_compiler_flag stores its result in CXX_SUPPORTS_NO_CXX98_COMPAT_EXTRA_SEMI_FLAG (with _NO_), but the guarding if() checked CXX_SUPPORTS_CXX98_COMPAT_EXTRA_SEMI_FLAG (without _NO_), which is never set. The condition was always false, so the flag was never applied. The three sibling blocks (-Wno-return-type-c-linkage, -Wno-nested-anon-types, -Wno-gnu-anonymous-struct) use matching names and work correctly.

Originally flagged by rocmlir-pr-reviewer[bot] on #2356 (https://github.com/ROCm/rocMLIR/pull/2402#issue-4613195205).

Technical Details

Fixed the if() guards to read the variable that is actually set (CXX_SUPPORTS_NO_CXX98_COMPAT_EXTRA_SEMI_FLAG) in three places:

• mlir/tools/rocmlir-tuning-driver/CMakeLists.txt:48 (rocMLIR-owned).
• external/llvm-project/mlir/lib/ExecutionEngine/CMakeLists.txt:424 and :467 ([EXTERNAL] commit). Line 424 mirrors the same bug present in upstream LLVM main; line 467 is a rocMLIR-added block.

Config-only change. No functional impact in the default build (the warning is only emitted under -Wc++98-compat/-Weverything); the fix ensures the suppression actually applies when those warnings are enabled on mlir_rocm_runtime / obj.MLIRRocmExecutionEngineUtils.

Test Plan

• Confirmed via repo-wide grep that the set and read variable names now match everywhere and no occurrence of the old (never-set) name remains.
• Relies on premerge CI to validate CMake configure/build.

Test Result

• All check_cxx_compiler_flag results are now consumed by a matching if(); the previously dead suppression is live.

Submission Checklist

• Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

[Copilot]

Pull request overview

This PR fixes a typo in CMake guard variables so the -Wno-c++98-compat-extra-semi warning suppression is actually applied when the compiler supports it, matching how the adjacent check_cxx_compiler_flag()-guarded suppressions already work.

Changes:

• Correct if() conditions to check CXX_SUPPORTS_NO_CXX98_COMPAT_EXTRA_SEMI_FLAG (the variable set by check_cxx_compiler_flag) instead of the never-set CXX_SUPPORTS_CXX98_COMPAT_EXTRA_SEMI_FLAG.
• Apply the same fix in rocMLIR-owned CMake plus two locations in the vendored LLVM MLIR ExecutionEngine CMake.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
mlir/tools/rocmlir-tuning-driver/CMakeLists.txt	Fixes the guard variable so mlir_rocm_runtime receives -Wno-c++98-compat-extra-semi when supported.
external/llvm-project/mlir/lib/ExecutionEngine/CMakeLists.txt	Fixes the same dead guard for mlir_rocm_runtime and obj.MLIRRocmExecutionEngineUtils so the suppression is applied when supported.

---

💡 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

Fixes a variable-name typo that made the -Wno-c++98-compat-extra-semi warning suppression dead code at three CMake sites: external/llvm-project/mlir/lib/ExecutionEngine/CMakeLists.txt:424 and :467, and mlir/tools/rocmlir-tuning-driver/CMakeLists.txt:48. The guarding if() now reads the same CXX_SUPPORTS_NO_CXX98_COMPAT_EXTRA_SEMI_FLAG that check_cxx_compiler_flag actually sets.

Findings

No blocking issues found.

Notes

• Diff is minimal and surgical — three identical one-token renames, all from the never-set name to the actually-set name. Verified the check_cxx_compiler_flag calls immediately above each site write CXX_SUPPORTS_NO_CXX98_COMPAT_EXTRA_SEMI_FLAG, so the guards now match.
• Sibling suppressions in the same files (-Wno-return-type-c-linkage, -Wno-nested-anon-types, -Wno-gnu-anonymous-struct) already use matching names, confirming the intended pattern.
• The external/ change is correctly isolated in its own [EXTERNAL]-prefixed commit (db30ea81b), separate from the rocMLIR-side commit (1ad6c33b4), per the checklist's Major rule about not mixing external/ with rocMLIR changes.
• Config-only change; no runtime behavior impact in the default build. The suppression only matters under -Wc++98-compat/-Weverything, where it now actually applies.
• The external/ line 424 block mirrors the same typo in upstream LLVM main; an upstream fix is out of scope here but worth flagging to upstream separately.

CI status

All non-pending checks are passing at review time; premerge C/C++, Python lint, and Python perf jobs are still in progress. No fail/cancel buckets observed.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2402      +/-   ##
===========================================
+ Coverage    82.57%   82.65%   +0.09%     
===========================================
  Files          120      120              
  Lines        42852    42852              
  Branches      7110     7110              
===========================================
+ Hits         35381    35418      +37     
+ Misses        4815     4811       -4     
+ Partials      2656     2623      -33     

Flag	Coverage Δ
gfx120x	82.55% <ø> (+0.03%)	⬆️
gfx950	82.39% <ø> (+0.04%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 33 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[umangyadav]

external/llvm-project/mlir/lib/ExecutionEngine/CMakeLists.txt:424 and :467 ([EXTERNAL] commit). Line 424 mirrors the same bug present in upstream LLVM main; line 467 is a rocMLIR-added block.

Can we create upstream PR to fix this ?

[umangyadav]

better to create upstream PR

[bogdan-petkovic]

I've opened an upstream PR for the same fix in llvm/llvm-project: llvm/llvm-project#204524 Note that this rocMLIR PR is independent of the upstream one, it also fixes the rocMLIR-added block (line 467) and the rocmlir-tuning-driver CMake, and it keeps our vendored copy correct regardless of when the upstream change lands. So I don't think we need to block on upstream. What do you think @umangyadav, OK to merge this one now?