[all-commits] [llvm/llvm-project] 594919: [mlir][spirv] Split conditional basic blocks durin...

[Igor Wodiany via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 594919c263122e1d0468dfecee6eb5962e892b44
      https://github.com/llvm/llvm-project/commit/594919c263122e1d0468dfecee6eb5962e892b44
  Author: Igor Wodiany <igor.wodiany at imgtec.com>
  Date:   2025-02-24 (Mon, 24 Feb 2025)

  Changed paths:
    M mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
    M mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
    M mlir/test/Target/SPIRV/selection.mlir
    A mlir/test/Target/SPIRV/selection.spv
    M mlir/test/lit.cfg.py

  Log Message:
  -----------
  [mlir][spirv] Split conditional basic blocks during deserialization (#127639)

With the current design some of the values are sank into a selection
region, despite them being also used outside that region. This is
because the current deserializer logic sinks the entire basic block
containing a conditional branch forming a header of a selection
construct, without accounting for some values being used outside. This
manifests as (for example):

```
<unknown>:0: error: 'spirv.Variable' op failed control flow structurization: it has uses outside of the enclosing selection/loop construct
<unknown>:0: note: see current operation: %0 = "spirv.Variable"()<{storage_class = #spirv.storage_class<Function>}> : () -> !spirv.ptr<vector<4xf32>, Function>
```

The proposed solution to this problem is to split the conditional basic
block into two, one block containing just the conditional branch, and
other the rest of instructions. By doing this, the logic that structures
selection regions, only sinks the comparison, keeping the rest of
instructions outside the selection region.

A SPIR-V test is required, as the problem can happen only during
deserialization and cannot be tested with `--test-spirv-roundtrip`. An
MLIR test exhibiting the problematic behaviour would be an incorrect
MLIR in the first place.

This solution is proposed as an alternative to an unfinished PR #123371,
that is unlikely to be merged in the foreseeable future, as the author
"stepped away from this for a time being". There is also a Discourse
thread:
https://discourse.llvm.org/t/spir-v-uses-outside-the-selection-region/84494
that tried to solicit some feedback on the topic.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications