@@ -2104,37 +2104,75 @@ LogicalResult ControlFlowStructurizer::structurize() {
21042104 // selection/loop. If so, they will be recorded within blockMergeInfo.
21052105 // We need to update the pointers there to the newly remapped ones so we can
21062106 // continue structurizing them later.
2107+ //
2108+ // We need to walk each block as constructBlocks do not include blocks
2109+ // internal to ops already structured within those blocks. It is not
2110+ // fully clear to me why the mergeInfo of blocks (yet to be structured)
2111+ // inside already structured selections/loops get invalidated and needs
2112+ // updating, however the following example code can cause a crash (depending
2113+ // on the structuring order), when the most inner selection is being
2114+ // structured after the outer selection and loop have been already
2115+ // structured:
2116+ //
2117+ // spirv.mlir.for {
2118+ // // ...
2119+ // spirv.mlir.selection {
2120+ // // ..
2121+ // // A selection region that hasn't been yet structured!
2122+ // // ..
2123+ // }
2124+ // // ...
2125+ // }
2126+ //
2127+ // If the loop gets structured after the outer selection, but before the
2128+ // inner selection. Moving the already structured selection inside the loop
2129+ // will invalidate the mergeInfo of the region that is not yet structured.
2130+ // Just going over constructBlocks will not check and updated header blocks
2131+ // inside the already structured selection region. Walking block fixes that.
2132+ //
2133+ // TODO: If structuring was done in a fixed order starting with inner
2134+ // most constructs this most likely not be an issue and the whole code
2135+ // section could be removed. However, with the current non-deterministic
2136+ // order this is not possible.
2137+ //
21072138 // TODO: The asserts in the following assumes input SPIR-V blob forms
21082139 // correctly nested selection/loop constructs. We should relax this and
21092140 // support error cases better.
2110- auto it = blockMergeInfo.find (block);
2111- if (it != blockMergeInfo.end ()) {
2112- // Use the original location for nested selection/loop ops.
2113- Location loc = it->second .loc ;
2114-
2115- Block *newHeader = mapper.lookupOrNull (block);
2116- if (!newHeader)
2117- return emitError (loc, " failed control flow structurization: nested "
2118- " loop header block should be remapped!" );
2119-
2120- Block *newContinue = it->second .continueBlock ;
2121- if (newContinue) {
2122- newContinue = mapper.lookupOrNull (newContinue);
2123- if (!newContinue)
2141+ auto updateMergeInfo = [&](Block *block) -> WalkResult {
2142+ auto it = blockMergeInfo.find (block);
2143+ if (it != blockMergeInfo.end ()) {
2144+ // Use the original location for nested selection/loop ops.
2145+ Location loc = it->second .loc ;
2146+
2147+ Block *newHeader = mapper.lookupOrNull (block);
2148+ if (!newHeader)
21242149 return emitError (loc, " failed control flow structurization: nested "
2125- " loop continue block should be remapped!" );
2150+ " loop header block should be remapped!" );
2151+
2152+ Block *newContinue = it->second .continueBlock ;
2153+ if (newContinue) {
2154+ newContinue = mapper.lookupOrNull (newContinue);
2155+ if (!newContinue)
2156+ return emitError (loc, " failed control flow structurization: nested "
2157+ " loop continue block should be remapped!" );
2158+ }
2159+
2160+ Block *newMerge = it->second .mergeBlock ;
2161+ if (Block *mappedTo = mapper.lookupOrNull (newMerge))
2162+ newMerge = mappedTo;
2163+
2164+ // The iterator should be erased before adding a new entry into
2165+ // blockMergeInfo to avoid iterator invalidation.
2166+ blockMergeInfo.erase (it);
2167+ blockMergeInfo.try_emplace (newHeader, loc, it->second .control , newMerge,
2168+ newContinue);
21262169 }
21272170
2128- Block *newMerge = it->second .mergeBlock ;
2129- if (Block *mappedTo = mapper.lookupOrNull (newMerge))
2130- newMerge = mappedTo;
2171+ return WalkResult::advance ();
2172+ };
21312173
2132- // The iterator should be erased before adding a new entry into
2133- // blockMergeInfo to avoid iterator invalidation.
2134- blockMergeInfo.erase (it);
2135- blockMergeInfo.try_emplace (newHeader, loc, it->second .control , newMerge,
2136- newContinue);
2137- }
2174+ if (block->walk (updateMergeInfo).wasInterrupted ())
2175+ return failure ();
21382176
21392177 // The structured selection/loop's entry block does not have arguments.
21402178 // If the function's header block is also part of the structured control
0 commit comments