Revert "[SimpleLoopUnswitch] Reland "Generalize the notion of trivial unswitching" (#204934)"#3150
Closed
adelejjeh wants to merge 1 commit into
Closed
Conversation
… unswitching" (llvm#204934)" This reverts commit e8b663b. Bisected as the cause of a rocPRIM block_reduce / device_reduce / device_segmented_reduce miscompile on gfx942 (MI300X), ROCm 7.15 (LCOMPILER-2401): block reduction at block size 128 returns ~50%-low results (e.g. val=128 expected=236). First-parent + in-merge bisect on amd-staging isolated this commit (good parent 053d75c, first-bad e8b663b). Reverting it on current amd-staging restores correct results (test_block_reduce 360/360; tip fails without the revert). Temporary revert to unblock ROCm; upstream fix tracked at llvm#204934.
Author
|
Closing this stopgap revert: the root cause is now fixed upstream in llvm#207047 (merged as 362cd64), which adds a convergence guard to trivial loop unswitching instead of reverting llvm#204934. That is the correct fix for the rocPRIM block/device reduce miscompile on gfx942 (LCOMPILER-2401) and will reach amd-staging via the next main-to-amd-staging integration merge, so this revert is no longer needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Temporary revert on amd-staging to unblock ROCm.
Upstream commit e8b663b ("[SimpleLoopUnswitch] Reland "Generalize the notion of trivial unswitching" (llvm#204934)") miscompiles rocPRIM block/device/segmented reduce on gfx942 (MI300X), ROCm 7.15 — bisected in LCOMPILER-2401.
block_reducewith a runtimevalid_itemscount at block size 128 returns ~50%-low results.Root cause: the new "trivial" unswitch case redirects a loop-invariant branch's latch edge to the loop exit; when the loop contains a convergent op (
llvm.amdgcn.update.dppwarp reduce) executed unconditionally on each iteration, hoisting the branch lets one (non-uniform) path bypass the loop and skip the convergent op, so lanes drop out of the cross-lane reduction.Verified: reverting this commit on current amd-staging restores correct results (
test_block_reduce360/360; tip fails without the revert). A minimalopt -passes=simple-loop-unswitchreproducer and lit test are attached to the upstream PR llvm#204934.This revert is a stopgap on amd-staging; the upstream fix (adding a convergence guard to the new trivial-unswitch case) is being pursued with the PR author.