[mlir] SymbolUserAttrInterface verification only sees top-level discardable attributes
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
`SymbolUserAttrInterface` (https://github.com/llvm/llvm-project/pull/153206) is only verified for top-level attributes within the discardable dictionary, consider this variant of that PR's negative test:
```mlir
module {
func.func @test() attributes {symbol_refs = [#test.symbol_ref_attr<@non_existent_symbol>]} { return }
}
```
On MLIR built today (~ToT), `mlir-opt` on the above produces no errors. However, remove the `[]` and drop the plural and we get the test currently in tree:
```mlir
module {
func.func @test() attributes {symbol_ref = #test.symbol_ref_attr<@non_existent_symbol>} { return }
}
```
which produces the following diagnostic:
```
symattr.mlir:2:3: error: 'func.func' op TestSymbolRefAttr::verifySymbolUses: '@non_existent_symbol' does not reference a valid symbol
func.func @test() attributes {symbol_ref = #test.symbol_ref_attr<@non_existent_symbol>} { return }
^
symattr.mlir:2:3: note: see current operation:
"func.func"() <{function_type = () -> (), sym_name = "test"}> ({
"func.return"() : () -> ()
}) {symbol_ref = #test.symbol_ref_attr<@non_existent_symbol>} : () -> ()
```
Machinery like `walkSymbolRefs` does walk all attributes recursively; I was surprised to find this discrepancy.
Related: #198435 (since reverted, #217959) went a different way looking for verification candidates.
While we're working towards an effective verification story there, I wanted to start a thread separately to discuss the current design point.
The `SymbolUserAttrInterface` purpose is verification, but it being ignored when not top-level is not part of its contract AFAICT.
Contributor guide
Research direction
Start by reproducing both MLIR snippets with mlir-opt, comparing the nested discardable-attribute case with the top-level case. Read the SymbolUserAttrInterface verification path and the related walkSymbolRefs behavior, then add or update the negative test so nested symbol references produce the expected diagnostic.
Written by the indexing model from the issue text.
Assessment
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100