Skip to content

Conversation

@Men-cotton
Copy link
Contributor

Fix crash in checkMappingSpec for zero iterations or block/grid dims

computeProduct now only runs when numParallelIterations is nonzero and we reject non‑positive block/grid sizes early, eliminating the transform.gpu map crash and surfacing a clear diagnostic (see added test in mlir/test/Dialect/GPU/transform-gpu-failing.mlir).

Fixes: #73562

@Men-cotton Men-cotton requested a review from fabianmcg as a code owner December 2, 2025 11:51
@github-actions
Copy link

github-actions bot commented Dec 2, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@github-actions
Copy link

github-actions bot commented Dec 2, 2025

🐧 Linux x64 Test Results

  • 7168 tests passed
  • 595 tests skipped

✅ The build succeeded and all tests passed.

@Men-cotton
Copy link
Contributor Author

Men-cotton commented Dec 2, 2025

I observe a discrepancy between the CI results and my local environment (Ubuntu 24.04 LTS on WSL2, RelWithDebInfo build).

In CI, the test fails early with requires statically sized, normalized forall op, while my local build bypasses this check and instead fails later in the pipeline with: Trying to map to fewer GPU threads than loop iterations but overprovisioning is not yet supported. Try additional tiling before mapping or map to more threads.

With that in mind, I pushed a follow-up commit to this PR, “Fix: revert error message change” (82379d1), so that this patch only focuses on fixing the crash and keeps the diagnostic text consistent with what CI currently reports.

@Men-cotton
Copy link
Contributor Author

Hi, @darkbuck @okkwon

I've submitted a fix for the crash in transform.gpu.map_nested_forall_to_threads.
Since we cannot reproduce the specific environment reported (Ubuntu 24.04 with RelWithDebInfo) in the GitHub CI, could you please test this patch and confirm if it resolves the crash on your side?

Thanks!

@Men-cotton
Copy link
Contributor Author

In the commit 799bb9b, I replaced llvm::map_to_vector with a manual loop ensures that the captured failed variable is correctly updated when dynamic values are encountered.

Previously, the side effect was lost, causing the function to return a vector of zeros instead of std::nullopt for dynamic bounds, leading to crashes in downstream users like transform.gpu.map_nested_forall_to_threads.

@Men-cotton
Copy link
Contributor Author

After some testing in my local environment, I found that the problem could be solved by leaving only the commit 799bb9b and deleting the others.

@darkbuck
Copy link
Contributor

darkbuck commented Dec 2, 2025

After some testing in my local environment, I found that the problem could be solved by leaving only the commit 799bb9b and deleting the others.

I verified that 799bb9b fixes that test failure. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[mlir] Dialect/GPU/transform-gpu-failing.mlir crashes with RelWithDebInfo

2 participants